From 8b6ba12457c1d58281193a9d21086cae7499dbab Mon Sep 17 00:00:00 2001 From: Mads Nylund Date: Wed, 17 Jan 2024 22:20:57 +0100 Subject: [PATCH 01/23] created endpoint for listing and destroying orders --- app/payment/serializers/__init__.py | 1 + app/payment/serializers/order.py | 26 ++++++++++++++++++++++++++ app/payment/views/order.py | 28 +++++++++++++++++++++++++--- 3 files changed, 52 insertions(+), 3 deletions(-) diff --git a/app/payment/serializers/__init__.py b/app/payment/serializers/__init__.py index 76e890ea..b44781f3 100644 --- a/app/payment/serializers/__init__.py +++ b/app/payment/serializers/__init__.py @@ -2,4 +2,5 @@ OrderSerializer, OrderCreateSerializer, VippsOrderSerialzer, + OrderListSerializer ) diff --git a/app/payment/serializers/order.py b/app/payment/serializers/order.py index 9e4e9f80..17ae9cca 100644 --- a/app/payment/serializers/order.py +++ b/app/payment/serializers/order.py @@ -1,10 +1,36 @@ import uuid from app.common.serializers import BaseModelSerializer +from app.content.serializers.user import DefaultUserSerializer +from app.content.models import Event from app.content.util.event_utils import create_vipps_order from app.payment.models.order import Order +class OrderEventSerializer(BaseModelSerializer): + class Meta: + model = Event + fields = ( + "id", + "title" + ) + + +class OrderListSerializer(BaseModelSerializer): + event = OrderEventSerializer(many=False) + user = DefaultUserSerializer(many=False) + + class Meta: + model = Order + fields = ( + "order_id", + "created_at", + "status", + "user", + "event" + ) + + class OrderSerializer(BaseModelSerializer): class Meta: model = Order diff --git a/app/payment/views/order.py b/app/payment/views/order.py index 822a4c97..35af67cc 100644 --- a/app/payment/views/order.py +++ b/app/payment/views/order.py @@ -4,19 +4,33 @@ from sentry_sdk import capture_exception from app.common.mixins import ActionMixin -from app.common.permissions import BasicViewPermission +from app.common.permissions import BasicViewPermission, is_admin_user from app.common.viewsets import BaseViewSet from app.content.models import Registration, User from app.payment.models import Order -from app.payment.serializers import OrderCreateSerializer, OrderSerializer +from app.common.pagination import BasePagination +from app.payment.serializers import ( + OrderCreateSerializer, + OrderSerializer, + OrderListSerializer +) from app.payment.util.order_utils import is_expired class OrderViewSet(BaseViewSet, ActionMixin): permission_classes = [BasicViewPermission] - serializer_class = OrderSerializer + serializer_class = OrderListSerializer + pagination_class = BasePagination queryset = Order.objects.all() + def list(self, request, *args, **kwargs): + if is_admin_user(request): + return super().list(request, *args, **kwargs) + return Response( + {"detail": "Du har ikke tilgang til å se disse ordrene."}, + status=status.HTTP_403_FORBIDDEN, + ) + def retrieve(self, request, pk): try: user = request.query_params.get("user_id") @@ -66,3 +80,11 @@ def create(self, request, *args, **kwargs): {"detail": "Fant ikke bruker."}, status=status.HTTP_404_NOT_FOUND, ) + + def destroy(self, request, *args, **kwargs): + if is_admin_user(request): + return super().destroy(request, *args, **kwargs) + return Response( + {"detail": "Du har ikke tilgang til å slette denne ordren."}, + status=status.HTTP_403_FORBIDDEN, + ) From 7143316bfcdbcfdd3e7cad3ec25158b43242bb9b Mon Sep 17 00:00:00 2001 From: Martin Clementz Date: Fri, 19 Jan 2024 12:27:01 +0100 Subject: [PATCH 02/23] Infrastructure as Code using terraform (#684) * feat: initial terraform * chore: add basic documentation * chore: more documentation * Added IaC setup for Lepton * added checov github action * chekov allow softfail * chore: database name * hack to allow azure container apps managed certificates * small changes * tweeking values * chore: revert changes to makefile * chore: make format --- .github/workflows/terraform.yml | 39 ++++ .gitignore | 3 + .terraform.lock.hcl | 41 ++++ Makefile | 32 +-- app/content/views/event.py | 4 +- app/payment/serializers/__init__.py | 2 +- app/payment/serializers/order.py | 15 +- app/payment/views/order.py | 8 +- docker-compose.dev.yml | 20 -- docker-compose.prod.yml | 20 -- infrastructure/CODEOWNERS | 1 + infrastructure/README.md | 124 +++++++++++ infrastructure/containers.tf | 323 ++++++++++++++++++++++++++++ infrastructure/database.tf | 52 +++++ infrastructure/inputs.tf | 75 +++++++ infrastructure/main.tf | 43 ++++ infrastructure/registry.tf | 9 + infrastructure/storage.tf | 10 + infrastructure/vnet.tf | 53 +++++ inputs.tf | 1 + main.tf | 42 ++++ 21 files changed, 843 insertions(+), 74 deletions(-) create mode 100644 .github/workflows/terraform.yml create mode 100644 .terraform.lock.hcl delete mode 100644 docker-compose.dev.yml delete mode 100644 docker-compose.prod.yml create mode 100644 infrastructure/CODEOWNERS create mode 100644 infrastructure/README.md create mode 100644 infrastructure/containers.tf create mode 100644 infrastructure/database.tf create mode 100644 infrastructure/inputs.tf create mode 100644 infrastructure/main.tf create mode 100644 infrastructure/registry.tf create mode 100644 infrastructure/storage.tf create mode 100644 infrastructure/vnet.tf create mode 120000 inputs.tf create mode 100644 main.tf diff --git a/.github/workflows/terraform.yml b/.github/workflows/terraform.yml new file mode 100644 index 00000000..934c96ea --- /dev/null +++ b/.github/workflows/terraform.yml @@ -0,0 +1,39 @@ +name: Validate Infrastructure + +on: + workflow_dispatch: + pull_request: + branches: + - dev + - master + +jobs: + checkov: + permissions: + contents: read # for actions/checkout to fetch code + security-events: write # for github/codeql-action/upload-sarif to upload SARIF results + + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - name: Checkov GitHub Action + uses: bridgecrewio/checkov-action@v12 + with: + # This will add both a CLI output to the console and create a results.sarif file + output_format: sarif + framework: terraform + soft_fail: true + output_file_path: results.sarif + + - name: Upload SARIF file + uses: github/codeql-action/upload-sarif@v2 + + # Results are generated only on a success or failure + # this is required since GitHub by default won't run the next step + # when the previous one has failed. Security checks that do not pass will 'fail'. + # An alternative is to add `continue-on-error: true` to the previous step + # Or 'soft_fail: true' to checkov. + if: success() || failure() + with: + sarif_file: results.sarif diff --git a/.gitignore b/.gitignore index 5bcf46ad..05184f48 100644 --- a/.gitignore +++ b/.gitignore @@ -26,3 +26,6 @@ mysite.log .coverage celerybeat-schedule + +.terraform +*.tfvars diff --git a/.terraform.lock.hcl b/.terraform.lock.hcl new file mode 100644 index 00000000..3e007d98 --- /dev/null +++ b/.terraform.lock.hcl @@ -0,0 +1,41 @@ +# This file is maintained automatically by "terraform init". +# Manual edits may be lost in future updates. + +provider "registry.terraform.io/hashicorp/azurerm" { + version = "3.76.0" + constraints = "> 3.68.0" + hashes = [ + "h1:eArCWwNEShXmVWS08Ocd3d8ptsjbAaMECifkIBacpyw=", + "zh:33c6b1559b012d03befeb8ee9cf5b88c31acd64983dd4f727a49a436008b5577", + "zh:36d3cfa7cf2079a102ffce05da2de41ecf263310544990471c19ee01b135ccf3", + "zh:4481cda6a541116a756e9b206336aac7142ad1a063abcb592d2e0767256e436f", + "zh:4a4692b35517989c8eb6c52bb5435dfc342412620971a4a9c33e55e782559a4b", + "zh:52f3557d1bb156a7b4dcede631b326180caf091e25ff3976935ca40cdb8fac02", + "zh:7e4ba6d55d1d3e40379a746cb8ecaf9562b8831e62380fe5212b845702b36e6b", + "zh:8155f211b8a3af57d5ec9be9c775007a63cee587df8dd5e264cb4c70e9e2bb36", + "zh:a04c9763aef2e5e7b33f0b7da8af2d9e6910560425b6bb6065e4655ba278a047", + "zh:ab2a335765de282db3e6f29bd036d7a1404f7df4358ffa42a72163ee0cb48ba4", + "zh:b5cf262592d7ed3960d5e9d0c04f99a2e055cc918fe5e5e509a4f777a5001605", + "zh:bd8e096ed5062d97c49d19fc3b826119ede1c14173e7d6081af3bec0589f2080", + "zh:f569b65999264a9416862bca5cd2a6177d94ccb0424f3a4ef424428912b9cb3c", + ] +} + +provider "registry.terraform.io/hashicorp/random" { + version = "3.5.1" + hashes = [ + "h1:VSnd9ZIPyfKHOObuQCaKfnjIHRtR7qTw19Rz8tJxm+k=", + "zh:04e3fbd610cb52c1017d282531364b9c53ef72b6bc533acb2a90671957324a64", + "zh:119197103301ebaf7efb91df8f0b6e0dd31e6ff943d231af35ee1831c599188d", + "zh:4d2b219d09abf3b1bb4df93d399ed156cadd61f44ad3baf5cf2954df2fba0831", + "zh:6130bdde527587bbe2dcaa7150363e96dbc5250ea20154176d82bc69df5d4ce3", + "zh:6cc326cd4000f724d3086ee05587e7710f032f94fc9af35e96a386a1c6f2214f", + "zh:78d5eefdd9e494defcb3c68d282b8f96630502cac21d1ea161f53cfe9bb483b3", + "zh:b6d88e1d28cf2dfa24e9fdcc3efc77adcdc1c3c3b5c7ce503a423efbdd6de57b", + "zh:ba74c592622ecbcef9dc2a4d81ed321c4e44cddf7da799faa324da9bf52a22b2", + "zh:c7c5cde98fe4ef1143bd1b3ec5dc04baf0d4cc3ca2c5c7d40d17c0e9b2076865", + "zh:dac4bad52c940cd0dfc27893507c1e92393846b024c5a9db159a93c534a3da03", + "zh:de8febe2a2acd9ac454b844a4106ed295ae9520ef54dc8ed2faf29f12716b602", + "zh:eab0d0495e7e711cca367f7d4df6e322e6c562fc52151ec931176115b83ed014", + ] +} diff --git a/Makefile b/Makefile index e83ebda4..7807594d 100644 --- a/Makefile +++ b/Makefile @@ -6,20 +6,20 @@ help: ## This help. .PHONY: start start: ## Start the webserver with docker on http://localhost:8000 - docker-compose up + docker compose up .PHONY: down down: ## Take down server - docker-compose down -v + docker compose down -v .PHONY: restart restart: ## Rebuild and start the server - docker-compose build + docker compose build make start .PHONY: fresh fresh: ## Perform a fresh build, install and start the server - docker-compose build + docker compose build make makemigrations make migrate make loaddata @@ -28,16 +28,16 @@ fresh: ## Perform a fresh build, install and start the server .PHONY: createsuperuser createsuperuser: ## Create a new django superuser - docker-compose run --rm web python manage.py createsuperuser + docker compose run --rm web python manage.py createsuperuser .PHONY: makemigrations makemigrations: ## Create migration files - docker-compose run --rm web python manage.py makemigrations + docker compose run --rm web python manage.py makemigrations .PHONY: migrate migrate: ## Run django migrations - docker-compose run --rm web python manage.py migrate ${args} + docker compose run --rm web python manage.py migrate ${args} .PHONY: migrations migrations: ## Create migration-files and migrate immediately @@ -46,23 +46,23 @@ migrations: ## Create migration-files and migrate immediately .PHONY: dumpdata dumpdata: ## Dump current data stored into ./app/fixture.json - docker-compose run --rm web python manage.py dumpdata -e admin -e auth.Permission -e contenttypes --indent=4 > ./app/fixture.json + docker compose run --rm web python manage.py dumpdata -e admin -e auth.Permission -e contenttypes --indent=4 > ./app/fixture.json .PHONY: loaddata loaddata: ## Load fixtures from ./app/fixture.json into the database - docker-compose run --rm web python manage.py loaddata ./app/fixture.json + docker compose run --rm web python manage.py loaddata ./app/fixture.json .PHONY: collectstatic collectstatic: ## Collect static files to a single location to be served in production - docker-compose run --rm web python manage.py collectstatic + docker compose run --rm web python manage.py collectstatic .PHONY: test test: ## Run test suite - docker-compose run --rm web pytest ${args} + docker compose run --rm web pytest ${args} .PHONY: cov cov: ## Check test coverage - docker-compose run --rm web pytest --cov-config=.coveragerc --cov=app + docker compose run --rm web pytest --cov-config=.coveragerc --cov=app .PHONY: format format: ## Format code and imports @@ -77,15 +77,15 @@ check: ## Check formatting, imports and linting .PHONY: black black: ## Format code only - docker-compose run --rm web black app/ ${args} --exclude migrations + docker compose run --rm web black app/ ${args} --exclude migrations .PHONY: isort isort: ## Format imports only - docker-compose run --rm web isort . ${args} + docker compose run --rm web isort . ${args} .PHONY: flake8 flake8: ## Fheck code style - docker-compose run --rm web flake8 app + docker compose run --rm web flake8 app .PHONY: pr pr: ## Pull Request format and checks @@ -96,4 +96,4 @@ pr: ## Pull Request format and checks .PHONY: shell shell: ## Open an interactive Django shell - docker-compose run --rm web python manage.py shell + docker compose run --rm web python manage.py shell \ No newline at end of file diff --git a/app/content/views/event.py b/app/content/views/event.py index eb4512ec..ecb52257 100644 --- a/app/content/views/event.py +++ b/app/content/views/event.py @@ -81,7 +81,9 @@ def _list_queryset(self): .filter(~Q(category=category)) .order_by("-start_date") ) - return self.queryset.filter(end_date__gte=time).filter(~Q(category=category)) + return self.queryset.filter(end_date__gte=time).filter( + ~Q(category=category) + ) return self.queryset.filter(end_date__gte=time) diff --git a/app/payment/serializers/__init__.py b/app/payment/serializers/__init__.py index b44781f3..a3f61f8a 100644 --- a/app/payment/serializers/__init__.py +++ b/app/payment/serializers/__init__.py @@ -2,5 +2,5 @@ OrderSerializer, OrderCreateSerializer, VippsOrderSerialzer, - OrderListSerializer + OrderListSerializer, ) diff --git a/app/payment/serializers/order.py b/app/payment/serializers/order.py index 17ae9cca..3455642e 100644 --- a/app/payment/serializers/order.py +++ b/app/payment/serializers/order.py @@ -1,8 +1,8 @@ import uuid from app.common.serializers import BaseModelSerializer -from app.content.serializers.user import DefaultUserSerializer from app.content.models import Event +from app.content.serializers.user import DefaultUserSerializer from app.content.util.event_utils import create_vipps_order from app.payment.models.order import Order @@ -10,10 +10,7 @@ class OrderEventSerializer(BaseModelSerializer): class Meta: model = Event - fields = ( - "id", - "title" - ) + fields = ("id", "title") class OrderListSerializer(BaseModelSerializer): @@ -22,13 +19,7 @@ class OrderListSerializer(BaseModelSerializer): class Meta: model = Order - fields = ( - "order_id", - "created_at", - "status", - "user", - "event" - ) + fields = ("order_id", "created_at", "status", "user", "event") class OrderSerializer(BaseModelSerializer): diff --git a/app/payment/views/order.py b/app/payment/views/order.py index 35af67cc..3ea77272 100644 --- a/app/payment/views/order.py +++ b/app/payment/views/order.py @@ -4,15 +4,15 @@ from sentry_sdk import capture_exception from app.common.mixins import ActionMixin +from app.common.pagination import BasePagination from app.common.permissions import BasicViewPermission, is_admin_user from app.common.viewsets import BaseViewSet from app.content.models import Registration, User from app.payment.models import Order -from app.common.pagination import BasePagination from app.payment.serializers import ( - OrderCreateSerializer, + OrderCreateSerializer, + OrderListSerializer, OrderSerializer, - OrderListSerializer ) from app.payment.util.order_utils import is_expired @@ -80,7 +80,7 @@ def create(self, request, *args, **kwargs): {"detail": "Fant ikke bruker."}, status=status.HTTP_404_NOT_FOUND, ) - + def destroy(self, request, *args, **kwargs): if is_admin_user(request): return super().destroy(request, *args, **kwargs) diff --git a/docker-compose.dev.yml b/docker-compose.dev.yml deleted file mode 100644 index 7c9c44e0..00000000 --- a/docker-compose.dev.yml +++ /dev/null @@ -1,20 +0,0 @@ -version: '3.8' - -# Config file for registry settings in Azure App Service dev - -services: - web: - image: leptondevregistry.azurecr.io/lepton:latest - container_name: web - ports: - - 8000:8000 - celery: - image: leptondevregistry.azurecr.io/lepton:latest - container_name: celery - entrypoint: [] - command: celery --app app worker --task-events --beat --loglevel info - rabbitmq: - image: rabbitmq:3.9.13 - container_name: rabbitmq - ports: - - 5672:5672 diff --git a/docker-compose.prod.yml b/docker-compose.prod.yml deleted file mode 100644 index e59b53ee..00000000 --- a/docker-compose.prod.yml +++ /dev/null @@ -1,20 +0,0 @@ -version: '3.8' - -# Config file for registry settings in Azure App Service production - -services: - web: - image: leptonregistry.azurecr.io/lepton:latest - container_name: web - ports: - - 8000:8000 - celery: - image: leptonregistry.azurecr.io/lepton:latest - container_name: celery - entrypoint: [] - command: celery --app app worker --task-events --beat --loglevel info - rabbitmq: - image: rabbitmq:3.9.13 - container_name: rabbitmq - ports: - - 5672:5672 diff --git a/infrastructure/CODEOWNERS b/infrastructure/CODEOWNERS new file mode 100644 index 00000000..c099a5ad --- /dev/null +++ b/infrastructure/CODEOWNERS @@ -0,0 +1 @@ +@martcl diff --git a/infrastructure/README.md b/infrastructure/README.md new file mode 100644 index 00000000..6d001056 --- /dev/null +++ b/infrastructure/README.md @@ -0,0 +1,124 @@ +# Infrastructre + +What brave souls are wandering around in these parts? Infrastructure might be a bit big and scary, but don't worry, we'll get through this together. After reading this, you'll be able to: + +- Understand the basic concepts of infrastructure as Code +- Understand basic terraform concepts +- Understand basic Azure concepts +- Be able to contribute to this infrastructure + +## Overview +First of all, IaC (infrastructre as code) is a way of managing infrastructure in a declerative way. Imagine you are a customer at a resturant. You don't go into the kitchen and tell the chef how to cook your food, you just tell the waiter what you want and the chef will make it for you. This is the same way IaC works. You tell the cloud provider what you want, and they will make it for you. We use terraform to do this. Terraform is a tool that allows us to write code that will be translated into infrastructure. This is done by writing code using the hashicorp language HCL (Hashicorp Configuration Language). **The code should be written in a way that is easy to understand and easy to read.** This is because the code itelf is the documentation. + +Now with that out of the way, let's get into the actual infrastructure. We use Azure as our cloud provider. This means that we use Azure to host our infrastructure. This documentation will not go into detail about how Azure works, but it will explain the basics of how we use it. + +### You need +- Terraform cli +- Azure cli + +### Contributing to the infrastructure +First of all, you need to create a service principal to use with terraform authentication to Azure. Look at the section "Setup from scratch" to see how to do this. + +We have multiple enviroments for our infra, `dev` and `pro`. Dev is used for development and correspond to api-dev.tihlde.org and pro is used for production and correspond to api.tihlde.org. When you are working on the infrastructure, you should always work in the `dev` environment. This is done by running the following command: + +```bash +terraform workspace select dev +terraform init +``` + +After selecting the correct enviroment, you must have the correct `terraform.tfvars` file in the root of the project. This file contains the variables that are used to configure the infrastructure. Ask some of the other developers for the correct values. When you have the correct values, you can run `terraform plan` to see what will be changed. + +> ⚠️ Don't run "terraform apply" locally. This will change the infrastructure in the cloud. This task should be done by Github Actions. Keep this as a rule of thumb. + +When you are done making changes, you can commit and push your changes to Github. This will trigger a Github Action that will run terraform plan. Inspect this plan to see if everything looks good. If it does, you can merge it to master. This will trigger another Github Action that will run terraform apply. This will change the infrastructure in the cloud. + + +### Setup from scratch +If you are setting up the infrastructure from scratch, you will need to do a few things. First of all, you will need to setup a storage account to store the terraform state. This is done by running the following command ([source](https://learn.microsoft.com/en-us/azure/developer/terraform/store-state-in-azure-storage?tabs=azure-cli)): + +```bash +#!/bin/bash +RESOURCE_GROUP_NAME=tfstate +STORAGE_ACCOUNT_NAME=tfstatetihlde # must be globaly unique +CONTAINER_NAME=tfstate + +# Create resource group +az group create --name $RESOURCE_GROUP_NAME --location norwayeast + +# Create storage account +az storage account create --resource-group $RESOURCE_GROUP_NAME --name $STORAGE_ACCOUNT_NAME --sku Standard_LRS --encryption-services blob + +# Wait for a while... Azure is hella slow + +# Create blob container +az storage container create --name $CONTAINER_NAME --account-name $STORAGE_ACCOUNT_NAME +``` + +With that out of the way, you will need to create a service principal to use with terraform authentication to Azure. This is done by running the following commands ([source](https://learn.microsoft.com/en-us/azure/developer/terraform/get-started-cloud-shell-bash?tabs=bash)): + +```bash +#!/bin/bash +export MSYS_NO_PATHCONV=1 + +SERVICE_PRINCIPAL_NAME=martin-terraform # Choose a name for the service principal, this is just an example +SUBSCRIPTION_ID=$(az account show --query id --output tsv) # if you have more subscriptions, do "az account list" to get the id of the subscription you want to use + +az ad sp create-for-rbac --name $SERVICE_PRINCIPAL_NAME --role Contributor --scopes /subscriptions/$SUBSCRIPTION_ID +``` + +The command with output something similar to this: + +```bash +{ + "appId": "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx", + "displayName": "martin-terraform", + "password": "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx", + "tenant": "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" +} +``` + +When working locally, the easiest way to add these values is to add them to the `~/.bashrc` file. This is done to simplify the terraform setup. Add the following lines to the `~/.bashrc` file: + + +```bash +export ARM_SUBSCRIPTION_ID="" +export ARM_TENANT_ID="" +export ARM_CLIENT_ID="" +export ARM_CLIENT_SECRET="" +``` + +Remember to run `source ~/.bashrc` after you have added these values.😉 + + +> Little recap on what we just did: +> +> - We created a storage account to store the terraform state +> - We created a service principal to authenticate against Azure +> - We added the values of the service principal to the `~/.bashrc` file + + +We are now ready to start working with terraform localy. We want to have a `dev` and `prod` environment. This is done by creating terraform workspaces. You can create a workspace by running the following command: + +```bash +terraform workspace new dev +terraform workspace new pro +``` + +Select the workspace you want to work in by running the following command: + +```bash +terraform workspace select dev +``` + +Try changing the infrastructre a bit and run `terraform plan` to see what will be changed. + + + + +!!!!⚠️ +Remember to delete your infra when you are done playing around with it. This is done by running the following command: + +```bash +terraform destroy +``` + diff --git a/infrastructure/containers.tf b/infrastructure/containers.tf new file mode 100644 index 00000000..4950fe02 --- /dev/null +++ b/infrastructure/containers.tf @@ -0,0 +1,323 @@ +resource "azurerm_log_analytics_workspace" "lepton" { + name = "logspace" + location = azurerm_resource_group.lepton.location + resource_group_name = azurerm_resource_group.lepton.name + sku = "PerGB2018" + retention_in_days = 30 + + tags = local.common_tags +} + +resource "azurerm_container_app_environment" "lepton" { + name = "lepton-container-enviroment" + location = azurerm_resource_group.lepton.location + resource_group_name = azurerm_resource_group.lepton.name + log_analytics_workspace_id = azurerm_log_analytics_workspace.lepton.id + + infrastructure_subnet_id = azurerm_subnet.containers.id + + tags = local.common_tags +} + +resource "azurerm_container_app" "lepton-api" { + name = "lepton-api" + container_app_environment_id = azurerm_container_app_environment.lepton.id + resource_group_name = azurerm_resource_group.lepton.name + revision_mode = "Single" + + lifecycle { + ignore_changes = [ ingress ] // Required to not delete the manually created custom domain since it is not possible to create a managed certificate for a custom domain with terraform + } + + secret { + name = "reg-passwd" + value = azurerm_container_registry.lepton.admin_password + } + + registry { + server = azurerm_container_registry.lepton.login_server + password_secret_name = "reg-passwd" + username = azurerm_container_registry.lepton.admin_username + } + + template { + min_replicas = var.lepton_api_min_replicas + max_replicas = var.lepton_api_max_replicas + + container { + name = "lepton-api" + image = "${azurerm_container_registry.lepton.login_server}/lepton:latest" + cpu = 1.0 + memory = "2Gi" + + env { + name = "DATABASE_HOST" + value = azurerm_mysql_flexible_server.lepton-database-server.fqdn + } + env { + name = "DATABASE_NAME" + value = azurerm_mysql_flexible_database.lepton-database.name + } + env { + name = "DATABASE_PASSWORD" + value = azurerm_mysql_flexible_server.lepton-database-server.administrator_password + } + env { + name = "DATABASE_USER" + value = azurerm_mysql_flexible_server.lepton-database-server.administrator_login + } + env { + name = "DATABASE_PORT" + value = 3306 + } + env { + name = "AZURE_STORAGE_CONNECTION_STRING" + value = azurerm_storage_account.lepton.primary_connection_string + } + env { + name = "DJANGO_SECRET" + value = random_string.django_secret.result + } + + env { + name = "EMAIL_HOST" + value = "smtp.gmail.com" + } + env { + name = "EMAIL_PASSWORD" + value = var.email_password + } + env { + name = "EMAIL_PORT" + value = 587 + } + env { + name = "EMAIL_USER" + value = var.email_user + } + env { + name = "SENTRY_DSN" + value = var.centry_dsn + } + env { + name = "CELERY_BROKER_URL" + value = "amqp://guest:guest@rabbitmq:5672" + } + env { + name = "VIPPS_TOKEN_URL" + value = var.vipps_token_url + } + env { + name = "VIPPS_SUBSCRIPTION_KEY" + value = var.vipps_subscription_key + } + env { + name = "VIPPS_CALLBACK_PREFIX" + value = var.vipps_callback_prefix + } + env { + name = "VIPPS_CLIENT_ID" + value = var.vipps_client_id + } + env { + name = "VIPPS_CLIENT_SECRET" + value = var.vipps_client_secret + } + env { + name = "VIPPS_COOKIE" + value = "" + } + env { + name = "VIPPS_FALLBACK" + value = var.vipps_fallback_url + } + env { + name = "VIPPS_FORCE_PAYMENT_URL" + value = var.vipps_force_payment_url + } + env { + name = "VIPPS_MERCHANT_SERIAL_NUMBER" + value = var.vipps_merchant_serial_number + } + env { + name = "VIPPS_ORDER_URL" + value = var.vipps_order_url + } + env { + name = "PROD" + value = var.debug + } + } + } + + + ingress { + target_port = 8000 + allow_insecure_connections = true + external_enabled = true + traffic_weight { + percentage = 100 + latest_revision = true + } + } + + tags = local.common_tags +} + +resource "azurerm_container_app" "rabbitmq" { + name = "rabbitmq" + container_app_environment_id = azurerm_container_app_environment.lepton.id + resource_group_name = azurerm_resource_group.lepton.name + revision_mode = "Single" + + template { + min_replicas = 1 + max_replicas = 1 + + container { + name = "rabbitmq" + image = "rabbitmq:3.9.13" + cpu = 0.25 + memory = "0.5Gi" + } + } + + ingress { + target_port = 5672 + transport = "tcp" + traffic_weight { + percentage = 100 + latest_revision = true + } + } + + tags = local.common_tags +} + +resource "azurerm_container_app" "celery" { + name = "celery" + container_app_environment_id = azurerm_container_app_environment.lepton.id + resource_group_name = azurerm_resource_group.lepton.name + revision_mode = "Single" + + + secret { + name = "reg-passwd" + value = azurerm_container_registry.lepton.admin_password + } + + registry { + server = azurerm_container_registry.lepton.login_server + password_secret_name = "reg-passwd" + username = azurerm_container_registry.lepton.admin_username + } + + template { + min_replicas = 1 + max_replicas = 1 + + container { + name = "celery" + image = "${azurerm_container_registry.lepton.login_server}/lepton:latest" + cpu = 0.25 + memory = "0.5Gi" + command = ["celery", "--app", "app", "worker", "--task-events", "--beat", "--loglevel", "info"] + + env { + name = "DATABASE_HOST" + value = azurerm_mysql_flexible_server.lepton-database-server.fqdn + } + env { + name = "DATABASE_NAME" + value = azurerm_mysql_flexible_database.lepton-database.name + } + env { + name = "DATABASE_PASSWORD" + value = azurerm_mysql_flexible_server.lepton-database-server.administrator_password + } + env { + name = "DATABASE_USER" + value = azurerm_mysql_flexible_server.lepton-database-server.administrator_login + } + env { + name = "DATABASE_PORT" + value = 3306 + } + env { + name = "AZURE_STORAGE_CONNECTION_STRING" + value = azurerm_storage_account.lepton.primary_connection_string + } + env { + name = "DJANGO_SECRET" + value = random_string.django_secret.result + } + + env { + name = "EMAIL_HOST" + value = "smtp.gmail.com" + } + env { + name = "EMAIL_PASSWORD" + value = var.email_password + } + env { + name = "EMAIL_PORT" + value = 587 + } + env { + name = "EMAIL_USER" + value = var.email_user + } + env { + name = "SENTRY_DSN" + value = var.centry_dsn + } + env { + name = "CELERY_BROKER_URL" + value = "amqp://guest:guest@rabbitmq:5672" + } + env { + name = "VIPPS_TOKEN_URL" + value = var.vipps_token_url + } + env { + name = "VIPPS_SUBSCRIPTION_KEY" + value = var.vipps_subscription_key + } + env { + name = "VIPPS_CALLBACK_PREFIX" + value = var.vipps_callback_prefix + } + env { + name = "VIPPS_CLIENT_ID" + value = var.vipps_client_id + } + env { + name = "VIPPS_CLIENT_SECRET" + value = var.vipps_client_secret + } + env { + name = "VIPPS_COOKIE" + value = "" + } + env { + name = "VIPPS_FALLBACK" + value = var.vipps_fallback_url + } + env { + name = "VIPPS_FORCE_PAYMENT_URL" + value = var.vipps_force_payment_url + } + env { + name = "VIPPS_MERCHANT_SERIAL_NUMBER" + value = var.vipps_merchant_serial_number + } + env { + name = "VIPPS_ORDER_URL" + value = var.vipps_order_url + } + } + } + + tags = local.common_tags +} diff --git a/infrastructure/database.tf b/infrastructure/database.tf new file mode 100644 index 00000000..82d007d7 --- /dev/null +++ b/infrastructure/database.tf @@ -0,0 +1,52 @@ +resource "azurerm_mysql_flexible_server" "lepton-database-server" { + name = "lepton-database-${terraform.workspace}" + resource_group_name = azurerm_resource_group.lepton.name + location = azurerm_resource_group.lepton.location + administrator_login = random_string.database_username.result + administrator_password = random_password.database_password.result + delegated_subnet_id = azurerm_subnet.database.id + private_dns_zone_id = azurerm_private_dns_zone.lepton.id + backup_retention_days = 7 + sku_name = local.database_sku[terraform.workspace] + geo_redundant_backup_enabled = false + version = "8.0.21" + zone = "1" + + storage { + iops = 360 + size_gb = 20 + } + + tags = local.common_tags + + depends_on = [azurerm_private_dns_zone_virtual_network_link.lepton] +} + +resource "azurerm_mysql_flexible_server_configuration" "sql_generate_invisible_primary_key" { + name = "sql_generate_invisible_primary_key" + resource_group_name = azurerm_resource_group.lepton.name + server_name = azurerm_mysql_flexible_server.lepton-database-server.name + value = "OFF" +} + +resource "azurerm_mysql_flexible_server_configuration" "require_secure_transport" { + name = "require_secure_transport" + resource_group_name = azurerm_resource_group.lepton.name + server_name = azurerm_mysql_flexible_server.lepton-database-server.name + value = "OFF" +} + +resource "azurerm_mysql_flexible_database" "lepton-database" { + name = "db" + resource_group_name = azurerm_resource_group.lepton.name + server_name = azurerm_mysql_flexible_server.lepton-database-server.name + charset = "utf8mb4" + collation = "utf8mb4_0900_ai_ci" +} + +locals { + database_sku = { + dev = "B_Standard_B1s" + pro = "B_Standard_B2s" + } +} diff --git a/infrastructure/inputs.tf b/infrastructure/inputs.tf new file mode 100644 index 00000000..81a1c347 --- /dev/null +++ b/infrastructure/inputs.tf @@ -0,0 +1,75 @@ +variable "vipps_client_secret" { + type = string + sensitive = true +} + +variable "vipps_client_id" { + type = string + sensitive = true +} + +variable "vipps_subscription_key" { + type = string + sensitive = true +} + +variable "vipps_callback_prefix" { + type = string +} + +variable "vipps_fallback_url" { + type = string +} + +variable "vipps_force_payment_url" { + type = string + sensitive = true +} +variable "vipps_order_url" { + type = string + sensitive = true +} + +variable "vipps_token_url" { + type = string + sensitive = true +} + +variable "centry_dsn" { + type = string + sensitive = true +} + +variable "email_user" { + type = string +} + +variable "email_password" { + type = string + sensitive = true +} + +variable "vipps_merchant_serial_number" { + type = string + sensitive = true +} + +variable "lepton_api_min_replicas" { + type = number + default = 1 +} + +variable "lepton_api_max_replicas" { + type = number + default = 1 +} + +variable "enviroment" { + type = string + description = "value is either dev or pro" + default = "dev" +} + +variable "debug" { + default = "false" +} diff --git a/infrastructure/main.tf b/infrastructure/main.tf new file mode 100644 index 00000000..95abb31c --- /dev/null +++ b/infrastructure/main.tf @@ -0,0 +1,43 @@ +resource "azurerm_resource_group" "lepton" { + name = "tihlde-${var.enviroment}" + location = "northeurope" + + tags = local.common_tags +} + +resource "random_password" "database_password" { + length = 18 + special = true + override_special = "!#$%&*()-_=+[]{}<>:?" +} + +resource "random_string" "database_username" { + length = 16 + special = false +} + +resource "random_string" "django_secret" { + length = 32 + special = true +} + +output "acr_admin_username" { + value = azurerm_container_registry.lepton.admin_username +} + +output "acr_admin_password" { + value = azurerm_container_registry.lepton.admin_password +} + +output "acr_login_server" { + value = azurerm_container_registry.lepton.login_server +} + +locals { + common_tags = { + environment = "${var.enviroment}" + workspace = "${terraform.workspace}" + team = "index" + managed_by = "terraform" + } +} diff --git a/infrastructure/registry.tf b/infrastructure/registry.tf new file mode 100644 index 00000000..fbfbd64c --- /dev/null +++ b/infrastructure/registry.tf @@ -0,0 +1,9 @@ +resource "azurerm_container_registry" "lepton" { + name = "leptonregistry${var.enviroment}" + resource_group_name = azurerm_resource_group.lepton.name + location = azurerm_resource_group.lepton.location + sku = "Basic" + admin_enabled = true + + tags = local.common_tags +} diff --git a/infrastructure/storage.tf b/infrastructure/storage.tf new file mode 100644 index 00000000..3af0ae0f --- /dev/null +++ b/infrastructure/storage.tf @@ -0,0 +1,10 @@ +resource "azurerm_storage_account" "lepton" { + name = "leptonstorage${var.enviroment}" + resource_group_name = azurerm_resource_group.lepton.name + location = azurerm_resource_group.lepton.location + account_tier = "Standard" + account_replication_type = "GRS" + min_tls_version = "TLS1_2" + + tags = local.common_tags +} diff --git a/infrastructure/vnet.tf b/infrastructure/vnet.tf new file mode 100644 index 00000000..f1901007 --- /dev/null +++ b/infrastructure/vnet.tf @@ -0,0 +1,53 @@ +resource "azurerm_network_security_group" "lepton" { + name = "database-security-group" + location = azurerm_resource_group.lepton.location + resource_group_name = azurerm_resource_group.lepton.name + + tags = local.common_tags +} + +resource "azurerm_virtual_network" "lepton" { + name = "lepton-network" + location = azurerm_resource_group.lepton.location + resource_group_name = azurerm_resource_group.lepton.name + address_space = ["10.0.0.0/16"] + + tags = local.common_tags +} + +resource "azurerm_subnet" "database" { + name = "database-subnet" + resource_group_name = azurerm_resource_group.lepton.name + virtual_network_name = azurerm_virtual_network.lepton.name + address_prefixes = ["10.0.8.0/21"] + + delegation { + name = "fs" + service_delegation { + name = "Microsoft.DBforMySQL/flexibleServers" + actions = [ + "Microsoft.Network/virtualNetworks/subnets/join/action", + ] + } + } +} + +resource "azurerm_subnet" "containers" { + name = "containers-subnet" + resource_group_name = azurerm_resource_group.lepton.name + virtual_network_name = azurerm_virtual_network.lepton.name + + address_prefixes = ["10.0.16.0/21"] +} + +resource "azurerm_private_dns_zone" "lepton" { + name = "leptondatabase.mysql.database.azure.com" + resource_group_name = azurerm_resource_group.lepton.name +} + +resource "azurerm_private_dns_zone_virtual_network_link" "lepton" { + name = "internal.tihlde.no" + private_dns_zone_name = azurerm_private_dns_zone.lepton.name + virtual_network_id = azurerm_virtual_network.lepton.id + resource_group_name = azurerm_resource_group.lepton.name +} diff --git a/inputs.tf b/inputs.tf new file mode 120000 index 00000000..e98f308b --- /dev/null +++ b/inputs.tf @@ -0,0 +1 @@ +infrastructure/inputs.tf \ No newline at end of file diff --git a/main.tf b/main.tf new file mode 100644 index 00000000..6eaf020c --- /dev/null +++ b/main.tf @@ -0,0 +1,42 @@ +terraform { + required_providers { + azurerm = { + source = "hashicorp/azurerm" + version = ">3.68.0" + } + } + backend "azurerm" { + resource_group_name = "devops" + storage_account_name = "tfstatetihlde" + container_name = "tfstate" + key = "lepton.tfstate" + } +} + +provider "azurerm" { + features {} +} + +module "infrastructure" { + source = "./infrastructure" + + email_password = var.email_password + email_user = var.email_user + + centry_dsn = var.centry_dsn + + enviroment = var.enviroment + + vipps_callback_prefix = var.vipps_callback_prefix + vipps_subscription_key = var.vipps_subscription_key + vipps_client_id = var.vipps_client_id + vipps_client_secret = var.vipps_client_secret + vipps_merchant_serial_number = var.vipps_merchant_serial_number + vipps_fallback_url = var.vipps_fallback_url + vipps_token_url = var.vipps_token_url + vipps_force_payment_url = var.vipps_force_payment_url + vipps_order_url = var.vipps_order_url + + lepton_api_min_replicas = var.lepton_api_min_replicas + lepton_api_max_replicas = var.lepton_api_max_replicas +} From f6d027a9e576555ef9407080e680665a2f7219b0 Mon Sep 17 00:00:00 2001 From: Martin Clementz Date: Fri, 19 Jan 2024 12:41:29 +0100 Subject: [PATCH 03/23] chore: bump build actions (#755) --- .github/workflows/deploy_to_azure.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/deploy_to_azure.yml b/.github/workflows/deploy_to_azure.yml index 28fc8704..1e1d8131 100644 --- a/.github/workflows/deploy_to_azure.yml +++ b/.github/workflows/deploy_to_azure.yml @@ -27,10 +27,10 @@ jobs: - name: Set up Docker Buildx id: buildx - uses: docker/setup-buildx-action@v1 + uses: docker/setup-buildx-action@v3 - name: Cache Docker layers - uses: actions/cache@v2 + uses: actions/cache@v4 with: path: /tmp/.buildx-cache key: ${{ runner.os }}-buildx-${{ github.sha }} @@ -39,7 +39,7 @@ jobs: - name: Build and push id: docker_build - uses: docker/build-push-action@v2 + uses: docker/build-push-action@v5 with: context: ./ file: ./compose/Dockerfile From e707c0665654a8f22396ddd8fa689d49b5c72aae Mon Sep 17 00:00:00 2001 From: Mads Nylund <73914541+MadsNyl@users.noreply.github.com> Date: Thu, 25 Jan 2024 12:56:47 +0100 Subject: [PATCH 04/23] Feat(payment)/orders (#757) * added filtersearch * added filter * added filter and listing * Add status field to the ordering filter and fix retrieve method in OrderViewSet * Refactor order filters and views * Add is_index_user function to check if user is in Index * Refactor order factory and serializers, add update endpoint for orders * Add admin group user permission to order views and tests --- app/common/permissions.py | 11 ++ app/payment/factories/order_factory.py | 4 +- app/payment/filters/order.py | 13 +++ app/payment/serializers/__init__.py | 1 + app/payment/serializers/order.py | 13 ++- app/payment/views/order.py | 62 ++++++++-- app/tests/payment/test_order_integration.py | 118 ++++++++++++++++++++ 7 files changed, 209 insertions(+), 13 deletions(-) create mode 100644 app/payment/filters/order.py create mode 100644 app/tests/payment/test_order_integration.py diff --git a/app/common/permissions.py b/app/common/permissions.py index c6599b2e..84d77af3 100644 --- a/app/common/permissions.py +++ b/app/common/permissions.py @@ -142,6 +142,17 @@ def is_admin_user(request): return check_has_access(AdminGroup.admin(), request) +def is_index_user(request): + set_user_id(request) + """Checks if user is in Index""" + user_id = request.user + + if user_id is None: + return False + + return check_has_access([AdminGroup.INDEX], request) + + def is_admin_group_user(request): set_user_id(request) """Checks if user is in HS, Index, Nok, Promo, Sosialen or Kok""" diff --git a/app/payment/factories/order_factory.py b/app/payment/factories/order_factory.py index bc842fa0..1c28c80a 100644 --- a/app/payment/factories/order_factory.py +++ b/app/payment/factories/order_factory.py @@ -1,5 +1,3 @@ -import random - import factory from factory.django import DjangoModelFactory @@ -15,5 +13,5 @@ class Meta: user = factory.SubFactory(UserFactory) event = factory.SubFactory(EventFactory) - status = random.choice([e.value for e in OrderStatus]) + status = OrderStatus.INITIATE payment_link = factory.Faker("url") diff --git a/app/payment/filters/order.py b/app/payment/filters/order.py new file mode 100644 index 00000000..ccfa229d --- /dev/null +++ b/app/payment/filters/order.py @@ -0,0 +1,13 @@ +from django_filters.rest_framework import FilterSet, OrderingFilter + +from app.payment.models import Order + + +class OrderFilter(FilterSet): + """Filters orders""" + + ordering = OrderingFilter(fields=("created_at",)) + + class Meta: + model = Order + fields = ["event", "status"] diff --git a/app/payment/serializers/__init__.py b/app/payment/serializers/__init__.py index a3f61f8a..b32657c6 100644 --- a/app/payment/serializers/__init__.py +++ b/app/payment/serializers/__init__.py @@ -3,4 +3,5 @@ OrderCreateSerializer, VippsOrderSerialzer, OrderListSerializer, + OrderUpdateSerializer, ) diff --git a/app/payment/serializers/order.py b/app/payment/serializers/order.py index 3455642e..3541023e 100644 --- a/app/payment/serializers/order.py +++ b/app/payment/serializers/order.py @@ -10,7 +10,7 @@ class OrderEventSerializer(BaseModelSerializer): class Meta: model = Event - fields = ("id", "title") + fields = ("id", "title", "image", "start_date", "end_date") class OrderListSerializer(BaseModelSerializer): @@ -23,9 +23,12 @@ class Meta: class OrderSerializer(BaseModelSerializer): + event = OrderEventSerializer(many=False) + user = DefaultUserSerializer(many=False) + class Meta: model = Order - fields = ("order_id", "status", "payment_link") + fields = ("order_id", "status", "payment_link", "created_at", "event", "user") class VippsOrderSerialzer(BaseModelSerializer): @@ -34,6 +37,12 @@ class Meta: fields = ("order_id",) +class OrderUpdateSerializer(BaseModelSerializer): + class Meta: + model = Order + fields = ("status",) + + class OrderCreateSerializer(BaseModelSerializer): class Meta: model = Order diff --git a/app/payment/views/order.py b/app/payment/views/order.py index 3ea77272..eedb43cd 100644 --- a/app/payment/views/order.py +++ b/app/payment/views/order.py @@ -1,18 +1,26 @@ -from rest_framework import status +from django_filters.rest_framework import DjangoFilterBackend +from rest_framework import filters, status from rest_framework.response import Response from sentry_sdk import capture_exception from app.common.mixins import ActionMixin from app.common.pagination import BasePagination -from app.common.permissions import BasicViewPermission, is_admin_user +from app.common.permissions import ( + BasicViewPermission, + is_admin_user, + is_admin_group_user, + is_index_user, +) from app.common.viewsets import BaseViewSet from app.content.models import Registration, User +from app.payment.filters.order import OrderFilter from app.payment.models import Order from app.payment.serializers import ( OrderCreateSerializer, OrderListSerializer, OrderSerializer, + OrderUpdateSerializer, ) from app.payment.util.order_utils import is_expired @@ -23,8 +31,18 @@ class OrderViewSet(BaseViewSet, ActionMixin): pagination_class = BasePagination queryset = Order.objects.all() + filter_backends = [DjangoFilterBackend, filters.SearchFilter] + filterset_class = OrderFilter + search_fields = [ + "order_id", + "event__title", + "user__first_name", + "user__last_name", + "user__user_id", + ] + def list(self, request, *args, **kwargs): - if is_admin_user(request): + if is_admin_group_user(request): return super().list(request, *args, **kwargs) return Response( {"detail": "Du har ikke tilgang til å se disse ordrene."}, @@ -33,11 +51,14 @@ def list(self, request, *args, **kwargs): def retrieve(self, request, pk): try: - user = request.query_params.get("user_id") - event = request.query_params.get("event") - orders = Order.objects.filter(user=user, event=event) + if not is_admin_group_user(request): + return Response( + {"detail": "Du har ikke tilgang til å se denne ordren."}, + status=status.HTTP_403_FORBIDDEN, + ) + order = Order.objects.get(order_id=pk) serializer = OrderSerializer( - orders, context={"request": request}, many=True + order, context={"request": request}, many=False ) return Response(serializer.data, status.HTTP_200_OK) except Order.DoesNotExist as order_not_exist: @@ -47,6 +68,31 @@ def retrieve(self, request, pk): status=status.HTTP_404_NOT_FOUND, ) + def update(self, request, pk): + try: + if not is_admin_user(request): + return Response( + {"detail": "Du har ikke tilgang til å oppdatere denne ordren."}, + status=status.HTTP_403_FORBIDDEN, + ) + order = Order.objects.get(order_id=pk) + serializer = OrderUpdateSerializer( + order, data=request.data, context={"request": request} + ) + if serializer.is_valid(): + order = super().perform_update(serializer) + serializer = OrderSerializer( + order, context={"request": request}, many=False + ) + return Response(serializer.data, status.HTTP_200_OK) + return Response(serializer.errors, status.HTTP_400_BAD_REQUEST) + except Order.DoesNotExist as order_not_exist: + capture_exception(order_not_exist) + return Response( + {"detail": "Fant ikke beatlingsordre."}, + status=status.HTTP_404_NOT_FOUND, + ) + def create(self, request, *args, **kwargs): try: user = request.user @@ -82,7 +128,7 @@ def create(self, request, *args, **kwargs): ) def destroy(self, request, *args, **kwargs): - if is_admin_user(request): + if is_index_user(request): return super().destroy(request, *args, **kwargs) return Response( {"detail": "Du har ikke tilgang til å slette denne ordren."}, diff --git a/app/tests/payment/test_order_integration.py b/app/tests/payment/test_order_integration.py new file mode 100644 index 00000000..d5544b82 --- /dev/null +++ b/app/tests/payment/test_order_integration.py @@ -0,0 +1,118 @@ +from rest_framework import status + +import pytest + +from app.common.enums import AdminGroup +from app.payment.enums import OrderStatus +from app.util.test_utils import add_user_to_group_with_name, get_api_client + +API_ORDERS_BASE_URL = "/payments/" + + +def get_orders_url_detail(order_id): + return f"{API_ORDERS_BASE_URL}{order_id}/" + + +@pytest.mark.django_db +def test_list_orders_as_anonymous_user(default_client): + """An anonymous user should not be able to list orders.""" + response = default_client.get(API_ORDERS_BASE_URL) + assert response.status_code == status.HTTP_403_FORBIDDEN + + +@pytest.mark.django_db +def test_list_orders_as_user(member): + """A user should not be able to list orders.""" + client = get_api_client(user=member) + response = client.get(API_ORDERS_BASE_URL) + assert response.status_code == status.HTTP_403_FORBIDDEN + + +@pytest.mark.django_db +@pytest.mark.parametrize("group_name", AdminGroup.all()) +def test_list_orders_as_admin_user(member, group_name): + """A member of an admin group should be able to list orders.""" + add_user_to_group_with_name(member, group_name) + client = get_api_client(user=member) + response = client.get(API_ORDERS_BASE_URL) + assert response.status_code == status.HTTP_200_OK + + +@pytest.mark.django_db +def test_retrieve_order_as_anonymous_user(default_client, order): + """An anonymous user should not be able to retrieve an order.""" + response = default_client.get(get_orders_url_detail(order.order_id)) + assert response.status_code == status.HTTP_403_FORBIDDEN + + +@pytest.mark.django_db +def test_retrieve_order_as_member(member, order): + """A user should not be able to retrieve an order.""" + client = get_api_client(user=member) + response = client.get(get_orders_url_detail(order.order_id)) + assert response.status_code == status.HTTP_403_FORBIDDEN + + +@pytest.mark.django_db +@pytest.mark.parametrize("group_name", AdminGroup.all()) +def test_retrieve_order_as_admin_user(member, order, group_name): + """A member of an adming group should be able to retrieve an order.""" + add_user_to_group_with_name(member, group_name) + client = get_api_client(user=member) + response = client.get(get_orders_url_detail(order.order_id)) + assert response.status_code == status.HTTP_200_OK + + +@pytest.mark.django_db +def test_delete_order_as_anonymous_user(default_client, order): + """An anonymous user should not be able to delete an order.""" + response = default_client.delete(get_orders_url_detail(order.order_id)) + assert response.status_code == status.HTTP_403_FORBIDDEN + + +@pytest.mark.django_db +def test_delete_order_as_member(member, order): + """A user should not be able to delete an order.""" + client = get_api_client(user=member) + response = client.delete(get_orders_url_detail(order.order_id)) + assert response.status_code == status.HTTP_403_FORBIDDEN + + +@pytest.mark.django_db +@pytest.mark.parametrize("group_name", [AdminGroup.INDEX]) +def test_delete_order_as_index_user(member, order, group_name): + """A index user should be able to delete an order.""" + add_user_to_group_with_name(member, group_name) + client = get_api_client(user=member) + response = client.delete(get_orders_url_detail(order.order_id)) + assert response.status_code == status.HTTP_204_NO_CONTENT + + +@pytest.mark.django_db +def test_update_order_as_anonymous_user(default_client, order): + """An anonymous user should not be able to update an order.""" + response = default_client.put(get_orders_url_detail(order.order_id)) + assert response.status_code == status.HTTP_403_FORBIDDEN + + +@pytest.mark.django_db +def test_update_order_as_member(member, order): + """A user should not be able to update an order.""" + client = get_api_client(user=member) + response = client.put(get_orders_url_detail(order.order_id)) + assert response.status_code == status.HTTP_403_FORBIDDEN + + +@pytest.mark.django_db +@pytest.mark.parametrize("group_name", [AdminGroup.INDEX]) +def test_update_order_as_index_user(member, order, group_name): + """A index user should be able to update an order.""" + add_user_to_group_with_name(member, group_name) + client = get_api_client(user=member) + data = {"status": OrderStatus.SALE} + response = client.put(get_orders_url_detail(order.order_id), data=data) + assert response.status_code == status.HTTP_200_OK + + order.refresh_from_db() + + assert order.status == OrderStatus.SALE From 528865420975a3c328816eed540abc0d52f4ac28 Mon Sep 17 00:00:00 2001 From: Mads Nylund <73914541+MadsNyl@users.noreply.github.com> Date: Fri, 2 Feb 2024 16:39:27 +0100 Subject: [PATCH 05/23] added permission checks for order model and removed from order viewset (#760) * added permission checks for order model and removed from order viewset * format --- app/content/serializers/user.py | 2 +- app/payment/models/order.py | 35 ++++++++++++++++++++- app/payment/views/order.py | 33 +------------------ app/tests/payment/test_order_integration.py | 8 ++--- 4 files changed, 40 insertions(+), 38 deletions(-) diff --git a/app/content/serializers/user.py b/app/content/serializers/user.py index 5efd06b8..d52aed01 100644 --- a/app/content/serializers/user.py +++ b/app/content/serializers/user.py @@ -174,7 +174,7 @@ def get_fields(self): class UserPermissionsSerializer(serializers.ModelSerializer): permissions = DRYGlobalPermissionsField( - actions=["write", "write_all", "read", "destroy"] + actions=["write", "write_all", "read", "destroy", "update", "retrieve"] ) class Meta: diff --git a/app/payment/models/order.py b/app/payment/models/order.py index 85dbef83..c44e7a08 100644 --- a/app/payment/models/order.py +++ b/app/payment/models/order.py @@ -3,7 +3,12 @@ from django.db import models from app.common.enums import AdminGroup -from app.common.permissions import BasePermissionModel +from app.common.permissions import ( + BasePermissionModel, + is_admin_group_user, + is_admin_user, + is_index_user, +) from app.content.models.event import Event from app.content.models.user import User from app.payment.enums import OrderStatus @@ -32,3 +37,31 @@ class Meta: def __str__(self): return f"{self.user} - {self.event.title} - {self.status} - {self.created_at}" + + @classmethod + def has_update_permission(cls, request): + return is_admin_user(request) + + @classmethod + def has_destroy_permission(cls, request): + return is_index_user(request) + + @classmethod + def has_retrieve_permission(cls, request): + return is_admin_group_user(request) + + @classmethod + def has_read_permission(cls, request): + return is_admin_group_user(request) + + def has_object_read_permission(self, request): + return self.has_read_permission(request) + + def has_object_update_permission(self, request): + return self.has_update_permission(request) + + def has_object_destroy_permission(self, request): + return self.has_destroy_permission(request) + + def has_object_retrieve_permission(self, request): + return self.has_retrieve_permission(request) diff --git a/app/payment/views/order.py b/app/payment/views/order.py index eedb43cd..ebe4f685 100644 --- a/app/payment/views/order.py +++ b/app/payment/views/order.py @@ -6,12 +6,7 @@ from app.common.mixins import ActionMixin from app.common.pagination import BasePagination -from app.common.permissions import ( - BasicViewPermission, - is_admin_user, - is_admin_group_user, - is_index_user, -) +from app.common.permissions import BasicViewPermission from app.common.viewsets import BaseViewSet from app.content.models import Registration, User from app.payment.filters.order import OrderFilter @@ -41,21 +36,8 @@ class OrderViewSet(BaseViewSet, ActionMixin): "user__user_id", ] - def list(self, request, *args, **kwargs): - if is_admin_group_user(request): - return super().list(request, *args, **kwargs) - return Response( - {"detail": "Du har ikke tilgang til å se disse ordrene."}, - status=status.HTTP_403_FORBIDDEN, - ) - def retrieve(self, request, pk): try: - if not is_admin_group_user(request): - return Response( - {"detail": "Du har ikke tilgang til å se denne ordren."}, - status=status.HTTP_403_FORBIDDEN, - ) order = Order.objects.get(order_id=pk) serializer = OrderSerializer( order, context={"request": request}, many=False @@ -70,11 +52,6 @@ def retrieve(self, request, pk): def update(self, request, pk): try: - if not is_admin_user(request): - return Response( - {"detail": "Du har ikke tilgang til å oppdatere denne ordren."}, - status=status.HTTP_403_FORBIDDEN, - ) order = Order.objects.get(order_id=pk) serializer = OrderUpdateSerializer( order, data=request.data, context={"request": request} @@ -126,11 +103,3 @@ def create(self, request, *args, **kwargs): {"detail": "Fant ikke bruker."}, status=status.HTTP_404_NOT_FOUND, ) - - def destroy(self, request, *args, **kwargs): - if is_index_user(request): - return super().destroy(request, *args, **kwargs) - return Response( - {"detail": "Du har ikke tilgang til å slette denne ordren."}, - status=status.HTTP_403_FORBIDDEN, - ) diff --git a/app/tests/payment/test_order_integration.py b/app/tests/payment/test_order_integration.py index d5544b82..df2cc14c 100644 --- a/app/tests/payment/test_order_integration.py +++ b/app/tests/payment/test_order_integration.py @@ -81,7 +81,7 @@ def test_delete_order_as_member(member, order): @pytest.mark.django_db @pytest.mark.parametrize("group_name", [AdminGroup.INDEX]) def test_delete_order_as_index_user(member, order, group_name): - """A index user should be able to delete an order.""" + """An index user should be able to delete an order.""" add_user_to_group_with_name(member, group_name) client = get_api_client(user=member) response = client.delete(get_orders_url_detail(order.order_id)) @@ -104,9 +104,9 @@ def test_update_order_as_member(member, order): @pytest.mark.django_db -@pytest.mark.parametrize("group_name", [AdminGroup.INDEX]) -def test_update_order_as_index_user(member, order, group_name): - """A index user should be able to update an order.""" +@pytest.mark.parametrize("group_name", [*AdminGroup.admin()]) +def test_update_order_as_admin_user(member, order, group_name): + """An index and HS user should be able to update an order.""" add_user_to_group_with_name(member, group_name) client = get_api_client(user=member) data = {"status": OrderStatus.SALE} From 7bd8fc8fe4e27048d0070e4c7276bd456ba2a512 Mon Sep 17 00:00:00 2001 From: Mads Nylund <73914541+MadsNyl@users.noreply.github.com> Date: Tue, 6 Feb 2024 14:51:28 +0100 Subject: [PATCH 06/23] fixed string representation for orders (#764) --- app/payment/models/order.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/payment/models/order.py b/app/payment/models/order.py index c44e7a08..f5c2454d 100644 --- a/app/payment/models/order.py +++ b/app/payment/models/order.py @@ -36,7 +36,7 @@ class Meta: ordering = ("-created_at",) def __str__(self): - return f"{self.user} - {self.event.title} - {self.status} - {self.created_at}" + return f"{self.user} - {self.event.title if self.event else ['slettet']} - {self.status} - {self.created_at}" @classmethod def has_update_permission(cls, request): From 89d874ba2b6915ceb23c0bc972d097bd1f073a5f Mon Sep 17 00:00:00 2001 From: Mads Nylund <73914541+MadsNyl@users.noreply.github.com> Date: Wed, 7 Feb 2024 16:32:32 +0100 Subject: [PATCH 07/23] =?UTF-8?q?removed=20bug=20that=20deleted=20paid=20e?= =?UTF-8?q?vent=20if=20event=20is=20updated.=20added=20more=20i=E2=80=A6?= =?UTF-8?q?=20(#765)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * removed bug that deleted paid event if event is updated. added more info to paid_event in adminpanel * format --- app/content/serializers/event.py | 23 ++++++++----------- app/payment/models/paid_event.py | 4 +++- app/tests/content/test_event_integration.py | 6 +++++ .../payment/test_paid_event_integration.py | 3 +++ 4 files changed, 22 insertions(+), 14 deletions(-) diff --git a/app/content/serializers/event.py b/app/content/serializers/event.py index be994abb..737b8a81 100644 --- a/app/content/serializers/event.py +++ b/app/content/serializers/event.py @@ -5,10 +5,6 @@ from app.common.enums import GroupType from app.common.serializers import BaseModelSerializer -from app.content.exceptions import ( - APIEventCantBeChangedToPaidEventException, - APIPaidEventCantBeChangedToFreeEventException, -) from app.content.models import Event, PriorityPool from app.content.serializers.priority_pool import ( PriorityPoolCreateSerializer, @@ -210,10 +206,10 @@ def update_queue(self, event, limit, instance_limit): if limit_difference < 0: event.move_users_from_queue_to_waiting_list(abs(limit_difference)) - def update_from_paid_to_free(self, event, paid_information_data): + def update_from_free_to_paid(self, event, paid_information_data): if paid_information_data and not event.is_paid_event: if event.has_participants: - raise APIEventCantBeChangedToPaidEventException() + return PaidEvent.objects.create( event=event, @@ -221,15 +217,16 @@ def update_from_paid_to_free(self, event, paid_information_data): paytime=paid_information_data["paytime"], ) - def update_from_free_to_paid(self, event, paid_information_data): + def update_from_paid_to_free(self, event, paid_information_data): if event.is_paid_event: - if not len(paid_information_data) and event.has_participants: - raise APIPaidEventCantBeChangedToFreeEventException() + if event.has_participants: + return - paid_event = PaidEvent.objects.filter(event=event) - if paid_event: - paid_event.first().delete() - event.paid_information = None + if not len(paid_information_data): + paid_event = PaidEvent.objects.filter(event=event) + if paid_event: + paid_event.first().delete() + event.paid_information = None def update_priority_pools(self, event, priority_pools_data): event.priority_pools.all().delete() diff --git a/app/payment/models/paid_event.py b/app/payment/models/paid_event.py index c77577a7..2a8ff965 100644 --- a/app/payment/models/paid_event.py +++ b/app/payment/models/paid_event.py @@ -21,4 +21,6 @@ class Meta: verbose_name_plural = "Paid_events" def __str__(self): - return f"Price: {self.price}" + return ( + f"Event: {self.event.title} - Price: {self.price} - Paytime: {self.paytime}" + ) diff --git a/app/tests/content/test_event_integration.py b/app/tests/content/test_event_integration.py index 66da8c27..12c508bf 100644 --- a/app/tests/content/test_event_integration.py +++ b/app/tests/content/test_event_integration.py @@ -875,6 +875,9 @@ def test_jubkom_has_not_create_permission(api_client, jubkom_member): @pytest.mark.django_db +@pytest.mark.skip( + reason="This is handled in the frontend. Should be refactored in backend." +) def test_update_from_free_event_with_participants_to_paid_event( api_client, admin_user, event, registration ): @@ -896,6 +899,9 @@ def test_update_from_free_event_with_participants_to_paid_event( @pytest.mark.django_db +@pytest.mark.skip( + reason="This is handled in the frontend. Should be refactored in backend." +) def test_update_from_paid_event_with_participants_to_free_event( api_client, admin_user, event, paid_event, registration ): diff --git a/app/tests/payment/test_paid_event_integration.py b/app/tests/payment/test_paid_event_integration.py index b6f62968..a0aaac87 100644 --- a/app/tests/payment/test_paid_event_integration.py +++ b/app/tests/payment/test_paid_event_integration.py @@ -136,6 +136,9 @@ def test_update_paid_event_as_admin(admin_user): @pytest.mark.django_db +@pytest.mark.skip( + reason="This is handled in the frontend. Should be refactored in backend." +) def test_update_paid_event_to_free_event_with_registrations_as_admin( admin_user, registration ): From 9e95ba74dfa5bc16c6e5f73e7be485ac58bf72d1 Mon Sep 17 00:00:00 2001 From: Mads Nylund <73914541+MadsNyl@users.noreply.github.com> Date: Wed, 7 Feb 2024 17:28:08 +0100 Subject: [PATCH 08/23] Update CHANGELOG.md (#766) --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8fd93fec..a11b22d6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,9 @@ ## Neste versjon +## Versjon 2023.02.07 +- 🦟 **Vipps** Brukere kan nå oppdatere betalt arrangement, uten at det betalte arrangementet blir slettet. + ## Versjon 2023.01.15 - ✨ **Reaksjoner** Brukere kan reagere med emojier på arrangementer og nyheter. - 🎨 **JubKom Rettigheter** Medlemmer av JubKom kan ikke lenger lage arrangementer. From 57d1735f43b030f25038731850c38d77a2dc4c2e Mon Sep 17 00:00:00 2001 From: Erik Skjellevik <98759397+eriskjel@users.noreply.github.com> Date: Wed, 7 Feb 2024 18:01:19 +0100 Subject: [PATCH 09/23] Feat(kontres)/initial setup (#720) * Initial setup * Update settings.py * created draft for reservation class and state enum * config for kontres * created endpoint for creating new reservation * created serializer for create_reservation * added create_reservation to url path * added admin.py to implement admin panel logic * added __Str__ for admin panel * added seperate class for a bookable item * made "Kontoret" default value of a reservation for now and added migration * removed unnecessary code * added endpoint to edit a reservation, lacking request validation * added endpoint to fetch all reservations * updated urls.py with the newest endpoints * code cleanup * created model test for reservation class * added bookable item object to reservation serializer * created test for creating reservations * added clean and self to reservation class * added bookable item serializer * added som error handling to fetch_all_reservations.py * created endpoint to fetch reseervation by id * modifies urls.py to accomodate changes in endpoints regarding queries and arguments * removed the default value for bookable item in reservation * every new reservation will now automatically be pending * fetch_reservation.py now uses url argument instead of query parameter * fixed bug in reservation_seralizer.py regarding bookableitem id * fixed and added more tests in test_create_reservation.py * created pytests for editing a reservation * created pytests for fetching all reservations * created pytests for fetching a reservation by 1 * made toString method in reservation model cleaner * combined files to make one common reservation endpoint * adjusted tests to accommodate to endpoint url * transitioned to uuid for reservation and bookable item class * fixed tests to accommodate uuid * created endpoint to fetch all bookable items * modified urls.py to accommodate uuid and new endpoint * removed old and seperate endpoint files * added uuid to admin panel * added error handling in reservation view * initial commit * all new reservations will be pending * added test for bookable items * deleted old endpoint and url model * fixed code for pr * fixed kontres conventions * formatting * added queryset to reservation model * fixed imports * rename * re-migrated * formatting * moved tests to correct place, and refactored to use factories and conftest.py * added reservation and bookable item factories to conftest.py * created factories * fixed migration issues * fixed packaging location * removed comments * added read and write access specifications to reservation and bookable item model * added endpoint guards * fixed tests * formatting * fixed tests * refactored permission system * refactored reservation model with correct permission system * fixed viewset to accomodate new permission system * removed unnecessary test * fixed old tests to accomodate new permission system, as well as added new ones * added extensive validation logic to prevent overlapping reservations etc * formatting for pr * removed relative import * linting * removed necessary code * rewrote reservation queryset * made reservation factory use enums for state * removed necessary state validation * fixed tests * formatting * linting * Added new field to reservation model * Trigger Build * Trigger Build * format and closed INSTALLED_APPS list * closed urls list * fixed description model bug as result of git issues * added class methods to bookable_item model * reformated permission logic * removed uneccessary code * translated error messages to norwegian * reformated tests to fit new viewset and permission logic * linting * changed from write to update permission and added status code on reponse on update view * creating a reservation will now use userId from request, and ignore any other attempt * users are now unable to modify reservation after it has been confirmed. also fixed permission logic in update method * added tests to make sure users cannot change their reservation after is has been confirmed * linting --------- Co-authored-by: Frikk Balder <33499052+MindChirp@users.noreply.github.com> Co-authored-by: ConradOsvik Co-authored-by: Mads Nylund <73914541+MadsNyl@users.noreply.github.com> Co-authored-by: Mads Nylund --- app/kontres/__init__.py | 0 app/kontres/admin.py | 16 + app/kontres/apps.py | 5 + app/kontres/enums.py | 7 + app/kontres/factories/__init__.py | 2 + .../factories/bookable_item_factory.py | 12 + app/kontres/factories/reservation_factory.py | 21 + app/kontres/migrations/0001_initial.py | 48 ++ ...remove_reservation_description_and_more.py | 22 + .../0003_reservation_description.py | 18 + app/kontres/migrations/__init__.py | 0 app/kontres/models/__init__.py | 0 app/kontres/models/bookable_item.py | 38 ++ app/kontres/models/reservation.py | 76 +++ app/kontres/serializer/__init__.py | 0 .../serializer/bookable_item_serializer.py | 9 + .../serializer/reservation_seralizer.py | 68 ++ app/kontres/urls.py | 13 + app/kontres/views/__init__.py | 0 app/kontres/views/bookable_item.py | 12 + app/kontres/views/reservation.py | 50 ++ app/settings.py | 1 + app/tests/conftest.py | 11 + app/tests/kontres/__init__.py | 0 .../kontres/test_reservation_integration.py | 600 ++++++++++++++++++ app/tests/kontres/test_reservation_model.py | 115 ++++ app/urls.py | 1 + 27 files changed, 1145 insertions(+) create mode 100644 app/kontres/__init__.py create mode 100644 app/kontres/admin.py create mode 100644 app/kontres/apps.py create mode 100644 app/kontres/enums.py create mode 100644 app/kontres/factories/__init__.py create mode 100644 app/kontres/factories/bookable_item_factory.py create mode 100644 app/kontres/factories/reservation_factory.py create mode 100644 app/kontres/migrations/0001_initial.py create mode 100644 app/kontres/migrations/0002_remove_reservation_description_and_more.py create mode 100644 app/kontres/migrations/0003_reservation_description.py create mode 100644 app/kontres/migrations/__init__.py create mode 100644 app/kontres/models/__init__.py create mode 100644 app/kontres/models/bookable_item.py create mode 100644 app/kontres/models/reservation.py create mode 100644 app/kontres/serializer/__init__.py create mode 100644 app/kontres/serializer/bookable_item_serializer.py create mode 100644 app/kontres/serializer/reservation_seralizer.py create mode 100644 app/kontres/urls.py create mode 100644 app/kontres/views/__init__.py create mode 100644 app/kontres/views/bookable_item.py create mode 100644 app/kontres/views/reservation.py create mode 100644 app/tests/kontres/__init__.py create mode 100644 app/tests/kontres/test_reservation_integration.py create mode 100644 app/tests/kontres/test_reservation_model.py diff --git a/app/kontres/__init__.py b/app/kontres/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/app/kontres/admin.py b/app/kontres/admin.py new file mode 100644 index 00000000..2b650b44 --- /dev/null +++ b/app/kontres/admin.py @@ -0,0 +1,16 @@ +from django.contrib import admin + +from app.kontres.models.bookable_item import BookableItem +from app.kontres.models.reservation import Reservation + + +class ReservationAdmin(admin.ModelAdmin): + readonly_fields = ("id",) + + +class BookableItemAdmin(admin.ModelAdmin): + readonly_fields = ("id",) + + +admin.site.register(Reservation, ReservationAdmin) +admin.site.register(BookableItem, BookableItemAdmin) diff --git a/app/kontres/apps.py b/app/kontres/apps.py new file mode 100644 index 00000000..5261dbe5 --- /dev/null +++ b/app/kontres/apps.py @@ -0,0 +1,5 @@ +from django.apps import AppConfig + + +class KontResConfig(AppConfig): + name = "app.kontres" diff --git a/app/kontres/enums.py b/app/kontres/enums.py new file mode 100644 index 00000000..44fc1670 --- /dev/null +++ b/app/kontres/enums.py @@ -0,0 +1,7 @@ +from django.db import models + + +class ReservationStateEnum(models.TextChoices): + PENDING = "PENDING" + CONFIRMED = "CONFIRMED" + CANCELLED = "CANCELLED" diff --git a/app/kontres/factories/__init__.py b/app/kontres/factories/__init__.py new file mode 100644 index 00000000..c3683ff6 --- /dev/null +++ b/app/kontres/factories/__init__.py @@ -0,0 +1,2 @@ +from app.kontres.factories.bookable_item_factory import BookableItemFactory +from app.kontres.factories.reservation_factory import ReservationFactory diff --git a/app/kontres/factories/bookable_item_factory.py b/app/kontres/factories/bookable_item_factory.py new file mode 100644 index 00000000..7accd2ee --- /dev/null +++ b/app/kontres/factories/bookable_item_factory.py @@ -0,0 +1,12 @@ +from factory import Faker, Sequence +from factory.django import DjangoModelFactory + +from app.kontres.models.bookable_item import BookableItem + + +class BookableItemFactory(DjangoModelFactory): + class Meta: + model = BookableItem + + name = Sequence(lambda n: f"Item_{n}") + description = Faker("text") diff --git a/app/kontres/factories/reservation_factory.py b/app/kontres/factories/reservation_factory.py new file mode 100644 index 00000000..791289b2 --- /dev/null +++ b/app/kontres/factories/reservation_factory.py @@ -0,0 +1,21 @@ +from django.utils import timezone + +from factory import Faker, SubFactory +from factory.django import DjangoModelFactory + +from app.content.factories import UserFactory +from app.kontres.enums import ReservationStateEnum +from app.kontres.factories.bookable_item_factory import BookableItemFactory +from app.kontres.models.reservation import Reservation + + +class ReservationFactory(DjangoModelFactory): + class Meta: + model = Reservation + + author = SubFactory(UserFactory) + bookable_item = SubFactory(BookableItemFactory) + start_time = timezone.now() + end_time = timezone.now() + timezone.timedelta(hours=1) + state = ReservationStateEnum.PENDING + description = Faker("text") diff --git a/app/kontres/migrations/0001_initial.py b/app/kontres/migrations/0001_initial.py new file mode 100644 index 00000000..843cf660 --- /dev/null +++ b/app/kontres/migrations/0001_initial.py @@ -0,0 +1,48 @@ +# Generated by Django 4.0.8 on 2023-10-25 13:10 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion +import uuid + + +class Migration(migrations.Migration): + + initial = True + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.CreateModel( + name='BookableItem', + fields=[ + ('created_at', models.DateTimeField(auto_now_add=True)), + ('updated_at', models.DateTimeField(auto_now=True)), + ('id', models.UUIDField(default=uuid.uuid4, editable=False, primary_key=True, serialize=False)), + ('name', models.CharField(max_length=20)), + ('description', models.TextField(blank=True)), + ], + options={ + 'abstract': False, + }, + ), + migrations.CreateModel( + name='Reservation', + fields=[ + ('created_at', models.DateTimeField(auto_now_add=True)), + ('updated_at', models.DateTimeField(auto_now=True)), + ('id', models.UUIDField(default=uuid.uuid4, editable=False, primary_key=True, serialize=False)), + ('start_time', models.DateTimeField()), + ('end_time', models.DateTimeField()), + ('state', models.CharField(choices=[('PENDING', 'Pending'), ('CONFIRMED', 'Confirmed'), ('CANCELLED', 'Cancelled')], default='PENDING', max_length=15)), + ('description', models.TextField(blank=True)), + ('author', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='reservations', to=settings.AUTH_USER_MODEL)), + ('bookable_item', models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, related_name='reservations', to='kontres.bookableitem')), + ], + options={ + 'abstract': False, + }, + ), + ] diff --git a/app/kontres/migrations/0002_remove_reservation_description_and_more.py b/app/kontres/migrations/0002_remove_reservation_description_and_more.py new file mode 100644 index 00000000..385be8fb --- /dev/null +++ b/app/kontres/migrations/0002_remove_reservation_description_and_more.py @@ -0,0 +1,22 @@ +# Generated by Django 4.2.5 on 2024-02-01 16:38 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("kontres", "0001_initial"), + ] + + operations = [ + migrations.RemoveField( + model_name="reservation", + name="description", + ), + migrations.AddField( + model_name="reservation", + name="accepted_rules", + field=models.BooleanField(default=True), + ), + ] diff --git a/app/kontres/migrations/0003_reservation_description.py b/app/kontres/migrations/0003_reservation_description.py new file mode 100644 index 00000000..e817ac48 --- /dev/null +++ b/app/kontres/migrations/0003_reservation_description.py @@ -0,0 +1,18 @@ +# Generated by Django 4.2.5 on 2024-02-07 11:40 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("kontres", "0002_remove_reservation_description_and_more"), + ] + + operations = [ + migrations.AddField( + model_name="reservation", + name="description", + field=models.TextField(blank=True), + ), + ] diff --git a/app/kontres/migrations/__init__.py b/app/kontres/migrations/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/app/kontres/models/__init__.py b/app/kontres/models/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/app/kontres/models/bookable_item.py b/app/kontres/models/bookable_item.py new file mode 100644 index 00000000..fc2ece6f --- /dev/null +++ b/app/kontres/models/bookable_item.py @@ -0,0 +1,38 @@ +import uuid + +from django.db import models + +from app.common.enums import AdminGroup, Groups +from app.common.permissions import BasePermissionModel, check_has_access +from app.util.models import BaseModel + + +class BookableItem(BaseModel, BasePermissionModel): + write_access = AdminGroup.admin() + read_access = [Groups.TIHLDE] + id = models.UUIDField(primary_key=True, default=uuid.uuid4, editable=False) + name = models.CharField(max_length=20) + description = models.TextField(blank=True) + + @classmethod + def has_read_permission(cls, request): + return check_has_access(cls.read_access, request) + + @classmethod + def has_retrieve_permission(cls, request): + return check_has_access(cls.read_access, request) + + @classmethod + def has_destroy_permission(cls, request): + return check_has_access(cls.write_access, request) + + @classmethod + def has_create_permission(cls, request): + return check_has_access(cls.write_access, request) + + @classmethod + def has_update_permission(cls, request): + return check_has_access(cls.write_access, request) + + def __str__(self): + return self.name diff --git a/app/kontres/models/reservation.py b/app/kontres/models/reservation.py new file mode 100644 index 00000000..9d205261 --- /dev/null +++ b/app/kontres/models/reservation.py @@ -0,0 +1,76 @@ +import uuid + +from django.db import models + +from app.common.enums import AdminGroup, Groups +from app.common.permissions import BasePermissionModel, check_has_access +from app.content.models import User +from app.kontres.enums import ReservationStateEnum +from app.kontres.models.bookable_item import BookableItem +from app.util.models import BaseModel + + +class Reservation(BaseModel, BasePermissionModel): + read_access = [Groups.TIHLDE] + write_access = [Groups.TIHLDE] + id = models.UUIDField(primary_key=True, default=uuid.uuid4, editable=False) + author = models.ForeignKey( + User, on_delete=models.CASCADE, related_name="reservations" + ) + bookable_item = models.ForeignKey( + BookableItem, on_delete=models.PROTECT, related_name="reservations" + ) + start_time = models.DateTimeField() + end_time = models.DateTimeField() + state = models.CharField( + max_length=15, + choices=ReservationStateEnum.choices, + default=ReservationStateEnum.PENDING, + ) + description = models.TextField(blank=True) + accepted_rules = models.BooleanField(default=True) + + def __str__(self): + return f"{self.state} - Reservation request by {self.author.first_name} {self.author.last_name} to book {self.bookable_item.name}. Created at {self.created_at}" + + @classmethod + def has_read_permission(cls, request): + return check_has_access(cls.read_access, request) + + @classmethod + def has_retrieve_permission(cls, request): + return check_has_access(cls.read_access, request) + + @classmethod + def has_update_permission(cls, request): + return check_has_access(cls.write_access, request) + + @classmethod + def has_destroy_permission(cls, request): + return check_has_access(cls.write_access, request) + + @classmethod + def has_create_permission(cls, request): + return check_has_access(cls.write_access, request) + + def has_object_update_permission(self, request): + allowed_groups = [AdminGroup.INDEX, AdminGroup.HS] + is_admin = check_has_access(allowed_groups, request) + + if ( + self.is_own_reservation(request) and "state" not in request.data + ) or is_admin: + return True + + if self.state == ReservationStateEnum.CONFIRMED and not is_admin: + return False + + # If trying to change the state, then check for admin permissions. + if "state" in request.data: + if request.data["state"] != self.state: + return check_has_access(allowed_groups, request) + + return False + + def is_own_reservation(self, request): + return self.author == request.user diff --git a/app/kontres/serializer/__init__.py b/app/kontres/serializer/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/app/kontres/serializer/bookable_item_serializer.py b/app/kontres/serializer/bookable_item_serializer.py new file mode 100644 index 00000000..96bff199 --- /dev/null +++ b/app/kontres/serializer/bookable_item_serializer.py @@ -0,0 +1,9 @@ +from rest_framework import serializers + +from app.kontres.models.bookable_item import BookableItem + + +class BookableItemSerializer(serializers.ModelSerializer): + class Meta: + model = BookableItem + fields = "__all__" diff --git a/app/kontres/serializer/reservation_seralizer.py b/app/kontres/serializer/reservation_seralizer.py new file mode 100644 index 00000000..7cb8ac9e --- /dev/null +++ b/app/kontres/serializer/reservation_seralizer.py @@ -0,0 +1,68 @@ +from django.db.models import Q +from django.utils import timezone +from rest_framework import serializers + +from app.kontres.models.bookable_item import BookableItem +from app.kontres.models.reservation import Reservation + + +class ReservationSerializer(serializers.ModelSerializer): + bookable_item = serializers.PrimaryKeyRelatedField( + queryset=BookableItem.objects.all() + ) + + class Meta: + model = Reservation + fields = "__all__" + + def validate_start_time(self, start_time): + if start_time < timezone.now(): + raise serializers.ValidationError("Start-tiden kan ikke være i fortiden.") + return start_time + + def validate(self, data): + + # Extract the bookable_item, start_time, and end_time, accounting for the possibility they may not be provided + bookable_item = data.get( + "bookable_item", self.instance.bookable_item if self.instance else None + ) + + # Validate the state change permission + if "state" in data: + if self.instance and data["state"] != self.instance.state: + user = self.context["request"].user + if not (user and user.is_authenticated and user.is_HS_or_Index_member): + raise serializers.ValidationError( + { + "state": "Du har ikke rettigheter til å endre reservasjonsstatusen." + } + ) + + # Validate that the end time is after the start time + start_time = data.get( + "start_time", self.instance.start_time if self.instance else None + ) + end_time = data.get( + "end_time", self.instance.end_time if self.instance else None + ) + if start_time and end_time and end_time <= start_time: + raise serializers.ValidationError("Slutt-tid må være etter start-tid") + + # Check for overlapping reservations only if necessary fields are present + if bookable_item and start_time and end_time: + # Build the query for overlapping reservations + overlapping_reservations_query = Q( + bookable_item=bookable_item, + end_time__gt=start_time, + start_time__lt=end_time, + ) + # Exclude the current instance if updating + if self.instance: + overlapping_reservations_query &= ~Q(pk=self.instance.pk) + # Check for overlapping reservations + if Reservation.objects.filter(overlapping_reservations_query).exists(): + raise serializers.ValidationError( + "Det er en reservasjonsoverlapp for det gitte tidsrommet." + ) + + return data diff --git a/app/kontres/urls.py b/app/kontres/urls.py new file mode 100644 index 00000000..f00e8baf --- /dev/null +++ b/app/kontres/urls.py @@ -0,0 +1,13 @@ +from django.urls import include, path +from rest_framework.routers import DefaultRouter + +from app.kontres.views.bookable_item import BookableItemViewSet +from app.kontres.views.reservation import ReservationViewSet + +router = DefaultRouter() +router.register(r"reservations", ReservationViewSet, basename="reservation") +router.register(r"bookable_items", BookableItemViewSet, basename="bookable_item") + +urlpatterns = [ + path("", include(router.urls)), +] diff --git a/app/kontres/views/__init__.py b/app/kontres/views/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/app/kontres/views/bookable_item.py b/app/kontres/views/bookable_item.py new file mode 100644 index 00000000..246937ca --- /dev/null +++ b/app/kontres/views/bookable_item.py @@ -0,0 +1,12 @@ +from app.common.permissions import BasicViewPermission +from app.common.viewsets import BaseViewSet +from app.kontres.models.bookable_item import BookableItem +from app.kontres.serializer.bookable_item_serializer import ( + BookableItemSerializer, +) + + +class BookableItemViewSet(BaseViewSet): + queryset = BookableItem.objects.all() + serializer_class = BookableItemSerializer + permission_classes = [BasicViewPermission] diff --git a/app/kontres/views/reservation.py b/app/kontres/views/reservation.py new file mode 100644 index 00000000..f48cd435 --- /dev/null +++ b/app/kontres/views/reservation.py @@ -0,0 +1,50 @@ +from django.db.models import Q +from django.utils.dateparse import parse_datetime +from rest_framework import status +from rest_framework.response import Response + +from app.common.permissions import BasicViewPermission +from app.common.viewsets import BaseViewSet +from app.kontres.models.reservation import Reservation +from app.kontres.serializer.reservation_seralizer import ReservationSerializer + + +class ReservationViewSet(BaseViewSet): + permission_classes = [BasicViewPermission] + serializer_class = ReservationSerializer + + def get_queryset(self): + start_date = self.request.GET.get("start_date") + end_date = self.request.GET.get("end_date") + + # Convert string dates to datetime objects + if start_date: + start_date = parse_datetime(start_date) + if end_date: + end_date = parse_datetime(end_date) + + # Adjusted filter to capture overlapping reservations + if start_date and end_date: + queryset = Reservation.objects.filter( + Q(start_time__lt=end_date) & Q(end_time__gt=start_date) + ) + return queryset + + return Reservation.objects.all() + + def create(self, request, *args, **kwargs): + request.data["author"] = request.user.user_id + serializer = ReservationSerializer(data=request.data) + if serializer.is_valid(): + # Overriding the state to PENDING + serializer.validated_data["state"] = "PENDING" + serializer.save() + return Response(serializer.data, status=status.HTTP_201_CREATED) + return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) + + def update(self, request, *args, **kwargs): + reservation = self.get_object() + serializer = self.get_serializer(reservation, data=request.data, partial=True) + serializer.is_valid(raise_exception=True) + serializer.save() + return Response(serializer.data, status=status.HTTP_200_OK) diff --git a/app/settings.py b/app/settings.py index 49137f7b..e44640aa 100644 --- a/app/settings.py +++ b/app/settings.py @@ -99,6 +99,7 @@ "app.gallery", "app.badge", "app.payment", + "app.kontres", "app.emoji", ] diff --git a/app/tests/conftest.py b/app/tests/conftest.py index 38c7ee57..02d22d5e 100644 --- a/app/tests/conftest.py +++ b/app/tests/conftest.py @@ -32,6 +32,7 @@ from app.group.factories import GroupFactory, MembershipFactory from app.group.factories.fine_factory import FineFactory from app.group.factories.membership_factory import MembershipHistoryFactory +from app.kontres.factories import BookableItemFactory, ReservationFactory from app.payment.factories.order_factory import OrderFactory from app.payment.factories.paid_event_factory import PaidEventFactory from app.util.test_utils import add_user_to_group_with_name, get_api_client @@ -243,6 +244,16 @@ def toddel(): return ToddelFactory() +@pytest.fixture() +def bookable_item(): + return BookableItemFactory() + + +@pytest.fixture() +def reservation(): + return ReservationFactory() + + @pytest.fixture() def news_reaction(member, news): return NewsReactionFactory(user=member, content_object=news) diff --git a/app/tests/kontres/__init__.py b/app/tests/kontres/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/app/tests/kontres/test_reservation_integration.py b/app/tests/kontres/test_reservation_integration.py new file mode 100644 index 00000000..83a20e05 --- /dev/null +++ b/app/tests/kontres/test_reservation_integration.py @@ -0,0 +1,600 @@ +from datetime import timedelta + +from django.utils import timezone +from rest_framework import status + +import pytest + +from app.common.enums import AdminGroup +from app.kontres.enums import ReservationStateEnum +from app.kontres.factories.bookable_item_factory import BookableItemFactory +from app.kontres.factories.reservation_factory import ReservationFactory +from app.kontres.models.bookable_item import BookableItem +from app.kontres.models.reservation import Reservation +from app.util.test_utils import get_api_client + + +@pytest.mark.django_db +def test_member_can_create_reservation(member, bookable_item): + client = get_api_client(user=member) + + response = client.post( + "/kontres/reservations/", + { + "bookable_item": bookable_item.id, + "start_time": "2030-10-10T10:00:00Z", + "end_time": "2030-10-10T11:00:00Z", + }, + format="json", + ) + + assert response.status_code == 201 + assert response.data["author"] == member.user_id + assert response.data["bookable_item"] == bookable_item.id + assert response.data["state"] == "PENDING" + + +@pytest.mark.django_db +def test_member_cannot_set_different_author_in_reservation(member, bookable_item): + client = get_api_client(user=member) + + # Attempt to create a reservation with a different author specified in the request body + response = client.post( + "/kontres/reservations/", + { + "author": "different_user_id", # Attempt to set a different author + "bookable_item": bookable_item.id, + "start_time": "2030-10-10T10:00:00Z", + "end_time": "2030-10-10T11:00:00Z", + }, + format="json", + ) + + # Check that the reservation is created successfully + assert response.status_code == 201 + + # Check that the author of the reservation is actually the requesting user + assert response.data["author"] == member.user_id + assert response.data["author"] != "different_user_id" + + # Check other attributes of the reservation + assert response.data["bookable_item"] == bookable_item.id + assert response.data["state"] == "PENDING" + + +@pytest.mark.django_db +def test_non_tihlde_cannot_create_reservation(user, bookable_item): + client = get_api_client(user=user) + + response = client.post( + "/kontres/reservations/", + { + "author": user.user_id, + "bookable_item": bookable_item.id, + "start_time": "2023-10-10T10:00:00Z", + "end_time": "2023-10-10T11:00:00Z", + }, + format="json", + ) + + assert response.status_code == 403 + + +@pytest.mark.django_db +def test_creating_reservation_with_past_start_time(member, bookable_item): + client = get_api_client(user=member) + past_time = timezone.now() - timezone.timedelta(days=1) + response = client.post( + "/kontres/reservations/", + { + "author": member.user_id, + "bookable_item": bookable_item.id, + "start_time": past_time, + "end_time": timezone.now(), + }, + format="json", + ) + assert response.status_code == status.HTTP_400_BAD_REQUEST + + +@pytest.mark.django_db +def test_member_deleting_own_reservation(member, reservation): + 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 + + +@pytest.mark.django_db +def test_user_cannot_create_confirmed_reservation(bookable_item, member): + client = get_api_client(user=member) + + # Set start_time to one hour from the current time + start_time = timezone.now() + timedelta(hours=1) + # Set end_time to two hours from the current time + end_time = timezone.now() + timedelta(hours=2) + + response = client.post( + "/kontres/reservations/", + { + "author": member.user_id, + "bookable_item": bookable_item.id, + # Format start_time and end_time to ISO format for the POST request + "start_time": start_time.isoformat(), + "end_time": end_time.isoformat(), + "state": "CONFIRMED", + }, + format="json", + ) + assert response.status_code == status.HTTP_201_CREATED + assert response.data["state"] == "PENDING" + + +@pytest.mark.django_db +def test_user_cannot_create_reservation_without_author(member, bookable_item): + client = get_api_client(user=member) + response = client.post( + "/kontres/reservations/", + { + "bookable_item_id": bookable_item.id, + "start_time": "2023-10-10T10:00:00Z", + "end_time": "2023-10-10T11:00:00Z", + }, + format="json", + ) + + assert response.status_code == 400 + + +@pytest.mark.django_db +def test_user_cannot_create_reservation_with_invalid_date_format(member, bookable_item): + client = get_api_client(user=member) + response = client.post( + "/kontres/reservations/", + { + "author": member.user_id, + "bookable_item_id": bookable_item.id, + "start_time": "invalid_date_format", + "end_time": "2023-10-10T11:00:00Z", + }, + format="json", + ) + + assert response.status_code == 400 + + +@pytest.mark.django_db +def test_admin_can_edit_reservation_to_confirmed(reservation, admin_user): + client = get_api_client(user=admin_user) + + response = client.put( + f"/kontres/reservations/{reservation.id}/", + {"state": "CONFIRMED"}, + format="json", + ) + + assert response.status_code == 200 + assert response.data["state"] == "CONFIRMED" + + +@pytest.mark.django_db +def test_admin_can_edit_reservation_to_cancelled(reservation, admin_user): + client = get_api_client(user=admin_user) + + response = client.put( + f"/kontres/reservations/{reservation.id}/", + {"state": "CANCELLED"}, + format="json", + ) + + assert response.status_code == 200 + assert response.data["state"] == "CANCELLED" + + +@pytest.mark.django_db +def test_updating_reservation_with_valid_times(member, reservation): + + reservation.author = member + reservation.save() + client = get_api_client(user=member) + + start_time = timezone.now() + timedelta(hours=1) + end_time = timezone.now() + timedelta(hours=2) + + response = client.put( + f"/kontres/reservations/{reservation.id}/", + { + "start_time": start_time.isoformat(), + "end_time": end_time.isoformat(), + }, + format="json", + ) + assert response.status_code == status.HTTP_200_OK + + # Parse the response times as timezone-aware datetimes + response_start_time = timezone.datetime.fromisoformat(response.data["start_time"]) + response_end_time = timezone.datetime.fromisoformat(response.data["end_time"]) + + # Ensure that the response_end_time is greater than response_start_time + assert response_end_time > response_start_time + + +@pytest.mark.django_db +def test_admin_cannot_edit_nonexistent_reservation(admin_user): + client = get_api_client(user=admin_user) + + nonexistent_uuid = "123e4567-e89b-12d3-a456-426655440000" + response = client.put( + f"/kontres/reservations/{nonexistent_uuid}/", + {"state": "CONFIRMED"}, + format="json", + ) + + assert response.status_code == 404 + + +@pytest.mark.django_db +def test_user_can_fetch_all_reservations(reservation, member): + client = get_api_client(user=member) + + reservations = [reservation] + for _ in range(2): + additional_reservation = ReservationFactory() + reservations.append(additional_reservation) + + response = client.get("/kontres/reservations/", format="json") + + assert response.status_code == 200 + assert len(response.data) == 3 + + first_reservation = Reservation.objects.first() + assert str(response.data[0]["id"]) == str(first_reservation.id) + assert response.data[0]["author"] == first_reservation.author.user_id + assert response.data[0]["bookable_item"] == first_reservation.bookable_item.id + assert response.data[0]["state"] == "PENDING" + + +@pytest.mark.django_db +def test_can_fetch_all_bookable_items(bookable_item, member): + client = get_api_client(user=member) + + bookable_items = [bookable_item] + for _ in range(2): + additional_bookable_item = BookableItemFactory() + bookable_items.append(additional_bookable_item) + + response = client.get("/kontres/bookable_items/", format="json") + + assert response.status_code == 200 + assert len(response.data) == 3 + + first_bookable_item = BookableItem.objects.first() + assert str(response.data[0]["id"]) == str(first_bookable_item.id) + assert response.data[0]["name"] == first_bookable_item.name + + +@pytest.mark.django_db +def test_user_can_fetch_bookable_items_when_none_exist(member): + client = get_api_client(user=member) + response = client.get("/kontres/bookable_items/", format="json") + + assert response.status_code == 200, response + + +@pytest.mark.django_db +def test_can_fetch_single_reservation(reservation, member): + client = get_api_client(user=member) + response = client.get(f"/kontres/reservations/{reservation.id}/", format="json") + + assert response.status_code == 200 + assert str(response.data["id"]) == str(reservation.id) + assert response.data["author"] == reservation.author.user_id + assert str(response.data["bookable_item"]) == str( + reservation.bookable_item.id + ) # Convert both to string + assert response.data["state"] == "PENDING" + + +@pytest.mark.django_db +def test_user_cannot_fetch_nonexistent_reservation(member): + client = get_api_client(user=member) + + non_existent_uuid = "12345678-1234-5678-1234-567812345678" + response = client.get(f"/kontres/reservations/{non_existent_uuid}/", format="json") + + assert response.status_code == 404 + + +@pytest.mark.django_db +def test_admin_can_delete_any_reservation(admin_user, reservation): + client = get_api_client(user=admin_user) + response = client.delete( + f"/kontres/reservations/{reservation.id}/", + format="json", + ) + assert response.status_code == status.HTTP_204_NO_CONTENT + + +@pytest.mark.django_db +def test_user_cannot_edit_others_reservation(user, reservation): + client = get_api_client(user=user) + reservation_id = str(reservation.id) + response = client.put( + f"/kontres/reservations/{reservation_id}/", + {"description": "New Description"}, + format="json", + ) + assert response.status_code == status.HTTP_403_FORBIDDEN + + +@pytest.mark.django_db +def test_user_cannot_delete_others_reservation(user, reservation): + client = get_api_client(user=user) + reservation_id = str(reservation.id) + response = client.delete( + f"/kontres/reservations/{reservation_id}/", + format="json", + ) + assert response.status_code == status.HTTP_403_FORBIDDEN + + +@pytest.mark.django_db +def test_admin_cannot_set_invalid_reservation_state(member, reservation): + client = get_api_client(user=member, group_name=AdminGroup.INDEX) + reservation_id = str(reservation.id) + response = client.put( + f"/kontres/reservations/{reservation_id}/", + {"state": "INVALID_STATE"}, + format="json", + ) + assert response.status_code == status.HTTP_400_BAD_REQUEST + + +@pytest.mark.django_db +def test_member_cannot_set_own_reservation_to_invalid_state(member, reservation): + reservation.author = member + reservation.save() + client = get_api_client(user=member) + reservation_id = str(reservation.id) + response = client.put( + f"/kontres/reservations/{reservation_id}/", + {"state": "INVALID_STATE"}, + format="json", + ) + assert response.status_code == status.HTTP_403_FORBIDDEN + + +@pytest.mark.django_db +def test_user_cannot_create_reservation_with_end_time_before_start_time( + member, bookable_item +): + client = get_api_client(user=member) + response = client.post( + "/kontres/reservations/", + { + "author": member.user_id, + "bookable_item": bookable_item.id, + "start_time": "2023-10-10T12:00:00Z", + "end_time": "2023-10-10T11:00:00Z", + }, + format="json", + ) + assert response.status_code == status.HTTP_400_BAD_REQUEST + + +@pytest.mark.django_db +def test_user_can_update_own_reservation_details(member, reservation): + reservation.author = member + reservation.save() + client = get_api_client(user=member) + reservation_id = str(reservation.id) + new_description = "Updated Description" + response = client.put( + f"/kontres/reservations/{reservation_id}/", + {"description": new_description}, + format="json", + ) + assert response.status_code == status.HTTP_200_OK + assert response.data["description"] == new_description + + +@pytest.mark.django_db +def test_unauthenticated_request_cannot_create_reservation(bookable_item): + client = get_api_client() + response = client.post( + "/kontres/reservations/", + { + "bookable_item": bookable_item.id, + "start_time": "2023-10-10T10:00:00Z", + "end_time": "2023-10-10T11:00:00Z", + }, + format="json", + ) + assert response.status_code == status.HTTP_403_FORBIDDEN + + +@pytest.mark.django_db +def test_creating_overlapping_reservation(member, bookable_item, admin_user): + # Create a confirmed reservation using the ReservationFactory + existing_confirmed_reservation = ReservationFactory( + bookable_item=bookable_item, + start_time=timezone.now() + timezone.timedelta(hours=1), + end_time=timezone.now() + timezone.timedelta(hours=2), + state=ReservationStateEnum.CONFIRMED, # Set the reservation as confirmed + ) + + # Now attempt to create an overlapping reservation + client = get_api_client(user=member) + overlapping_start_time = ( + existing_confirmed_reservation.start_time + timezone.timedelta(minutes=30) + ) + response = client.post( + "/kontres/reservations/", + { + "author": member.user_id, + "bookable_item": bookable_item.id, + "start_time": overlapping_start_time, + "end_time": existing_confirmed_reservation.end_time + + timezone.timedelta(hours=1), + "state": ReservationStateEnum.PENDING, + }, + format="json", + ) + + # The system should not allow this, as it overlaps with a confirmed reservation + assert response.status_code == status.HTTP_400_BAD_REQUEST + + +@pytest.mark.django_db +def test_retrieve_specific_reservation_within_its_date_range(member, bookable_item): + client = get_api_client(user=member) + + # Create a reservation with the current time + reservation = ReservationFactory( + author=member, + bookable_item=bookable_item, + start_time=timezone.now(), + end_time=timezone.now() + timezone.timedelta(hours=1), + ) + + # Broaden the query time range significantly for debugging + start_time = reservation.start_time - timezone.timedelta(hours=1) + end_time = reservation.end_time + timezone.timedelta(hours=1) + + # Format the start and end times in ISO 8601 format + start_time_iso = start_time.isoformat() + end_time_iso = end_time.isoformat() + + response = client.get( + f"/kontres/reservations/?start_date={start_time_iso}&end_date={end_time_iso}" + ) + + assert response.status_code == status.HTTP_200_OK + assert any(res["id"] == str(reservation.id) for res in response.data) + + +@pytest.mark.django_db +def test_retrieve_subset_of_reservations(member, bookable_item): + client = get_api_client(user=member) + + # Create three reservations with different times + # Use current time as a base to ensure consistency + current_time = timezone.now() + + times = [ + ( + current_time.replace(hour=10, minute=0, second=0, microsecond=0), + current_time.replace(hour=11, minute=0, second=0, microsecond=0), + ), + ( + current_time.replace(hour=10, minute=0, second=0, microsecond=0) + + timedelta(days=1), + current_time.replace(hour=11, minute=0, second=0, microsecond=0) + + timedelta(days=1), + ), + ( + current_time.replace(hour=10, minute=0, second=0, microsecond=0) + + timedelta(days=2), + current_time.replace(hour=11, minute=0, second=0, microsecond=0) + + timedelta(days=2), + ), + ] + + for start_time, end_time in times: + client.post( + "/kontres/reservations/", + { + "author": member.user_id, + "bookable_item": bookable_item.id, + "start_time": start_time.isoformat(), + "end_time": end_time.isoformat(), + }, + format="json", + ) + + # Define the query date range to include only the first two reservations + query_start_date = current_time.replace( + hour=9, minute=0, second=0, microsecond=0 + ).isoformat() + query_end_date = current_time.replace( + hour=9, minute=0, second=0, microsecond=0, day=current_time.day + 2 + ).isoformat() + + # Retrieve reservations for the specific date range + response = client.get( + f"/kontres/reservations/?start_date={query_start_date}&end_date={query_end_date}" + ) + + assert response.status_code == status.HTTP_200_OK + assert len(response.data) == 2 + + +@pytest.mark.django_db +def test_admin_can_update_confirmed_reservation_state(admin_user, reservation): + client = get_api_client(user=admin_user) + # Set the reservation state to CONFIRMED and save + reservation.state = ReservationStateEnum.CONFIRMED + reservation.save() + + new_state = "CANCELLED" + + response = client.put( + f"/kontres/reservations/{reservation.id}/", + {"state": new_state}, + format="json", + ) + + assert response.status_code == 200 + assert response.data["state"] == new_state + + +@pytest.mark.django_db +def test_user_cannot_update_confirmed_reservation(member, reservation): + client = get_api_client(user=member) + # Confirm the reservation before the test + reservation.state = ReservationStateEnum.CONFIRMED + reservation.save() + + response = client.patch( + f"/kontres/reservations/{reservation.id}/", + {"description": "Updated description"}, + format="json", + ) + + # Assuming 403 is the status code for a forbidden action + assert response.status_code == 403 + + +@pytest.mark.django_db +def test_member_can_update_own_reservation(member, reservation): + client = get_api_client(user=member) + + reservation.author = member + reservation.save() + + new_description = "Updated description" + response = client.patch( + f"/kontres/reservations/{reservation.id}/", + {"description": new_description}, + format="json", + ) + + assert response.status_code == 200 + assert response.data["description"] == new_description + + +@pytest.mark.django_db +def test_admin_can_update_details_of_confirmed_reservation(admin_user, reservation): + client = get_api_client(user=admin_user) + + reservation.state = ReservationStateEnum.CONFIRMED + reservation.save() + + new_description = "New details after confirmation" + response = client.patch( + f"/kontres/reservations/{reservation.id}/", + {"description": new_description}, + format="json", + ) + + assert response.status_code == 200 + assert response.data["description"] == new_description diff --git a/app/tests/kontres/test_reservation_model.py b/app/tests/kontres/test_reservation_model.py new file mode 100644 index 00000000..aff4e80f --- /dev/null +++ b/app/tests/kontres/test_reservation_model.py @@ -0,0 +1,115 @@ +from django.db.utils import IntegrityError +from django.utils import timezone + +import pytest + +from app.content.models import User +from app.kontres.models.bookable_item import BookableItem +from app.kontres.models.reservation import Reservation, ReservationStateEnum + + +@pytest.fixture() +def reservation(): + user = User.objects.create(user_id="test_user") + bookable_item = BookableItem.objects.create(name="Test Item") + return Reservation.objects.create( + author=user, + bookable_item=bookable_item, + start_time=timezone.now(), + end_time=timezone.now() + timezone.timedelta(hours=1), + ) + + +@pytest.mark.django_db +def test_reservation_defaults_to_pending(reservation): + assert reservation.state == ReservationStateEnum.PENDING + + +@pytest.mark.django_db +def test_reservation_start_and_end_time(): + user = User.objects.create(user_id="test_user", email="test@test.com") + bookable_item = BookableItem.objects.create(name="Test Item") + start_time = timezone.now() + end_time = start_time + timezone.timedelta(hours=1) + reservation = Reservation.objects.create( + author=user, + bookable_item=bookable_item, + start_time=start_time, + end_time=end_time, + ) + assert reservation.start_time == start_time + assert reservation.end_time == end_time + + +@pytest.mark.django_db +def test_state_transitions(reservation): + """Should correctly transition between states.""" + + # Start with a PENDING reservation + assert reservation.state == ReservationStateEnum.PENDING + + # Move to CONFIRMED + reservation.state = ReservationStateEnum.CONFIRMED + reservation.save() + assert reservation.state == ReservationStateEnum.CONFIRMED + + # Move to CANCELLED + reservation.state = ReservationStateEnum.CANCELLED + reservation.save() + assert reservation.state == ReservationStateEnum.CANCELLED + + +@pytest.mark.django_db +def test_reservation_requires_bookable_item(): + with pytest.raises(IntegrityError): + user = User.objects.create(user_id="test_user", email="test@test.com") + Reservation.objects.create( + start_time=timezone.now(), + end_time=timezone.now() + timezone.timedelta(hours=1), + author=user, + ) + + +@pytest.mark.django_db +def test_created_at_field(): + user = User.objects.create(user_id="test_user", email="test@test.com") + bookable_item = BookableItem.objects.create(name="Test Item") + reservation = Reservation.objects.create( + author=user, + bookable_item=bookable_item, + start_time=timezone.now(), + end_time=timezone.now() + timezone.timedelta(hours=1), + ) + assert reservation.created_at is not None + + +@pytest.mark.django_db +def test_multiple_reservations(): + user1 = User.objects.create(user_id="test_user_1", email="test1@test.com") + user2 = User.objects.create(user_id="test_user_2", email="test2@test.com") + bookable_item = BookableItem.objects.create(name="Test Item") + reservation1 = Reservation.objects.create( + author=user1, + bookable_item=bookable_item, + start_time=timezone.now(), + end_time=timezone.now() + timezone.timedelta(hours=1), + ) + reservation2 = Reservation.objects.create( + author=user2, + bookable_item=bookable_item, + start_time=timezone.now(), + end_time=timezone.now() + timezone.timedelta(hours=1), + ) + assert reservation1 is not None + assert reservation2 is not None + + +@pytest.mark.django_db +def test_reservation_requires_author(): + with pytest.raises(IntegrityError): + bookable_item = BookableItem.objects.create(name="Test Item") + Reservation.objects.create( + start_time=timezone.now(), + end_time=timezone.now() + timezone.timedelta(hours=1), + bookable_item=bookable_item, + ) diff --git a/app/urls.py b/app/urls.py index 9dd10e08..3e8029cd 100644 --- a/app/urls.py +++ b/app/urls.py @@ -31,5 +31,6 @@ path("forms/", include("app.forms.urls")), path("galleries/", include("app.gallery.urls")), path("badges/", include("app.badge.urls")), + path("kontres/", include("app.kontres.urls")), path("emojis/", include("app.emoji.urls")), ] From b82fd6a9e4a84533bf71f39bea648d9d09c8b879 Mon Sep 17 00:00:00 2001 From: Mads Nylund <73914541+MadsNyl@users.noreply.github.com> Date: Wed, 21 Feb 2024 17:27:48 +0100 Subject: [PATCH 10/23] LogEntry viewset and fix (#768) added viewset and serializer for logentry. Also remove date_hierachy in admin register for LogEntry --- app/content/admin/admin.py | 3 +- app/content/factories/__init__.py | 1 + app/content/factories/logentry_factory.py | 19 ++++ app/content/serializers/content_type.py | 15 ++++ app/content/serializers/logentry.py | 31 +++++++ app/content/urls.py | 2 + app/content/views/__init__.py | 1 + app/content/views/logentry.py | 39 ++++++++ .../content/test_logentry_integration.py | 89 +++++++++++++++++++ 9 files changed, 199 insertions(+), 1 deletion(-) create mode 100644 app/content/factories/logentry_factory.py create mode 100644 app/content/serializers/content_type.py create mode 100644 app/content/serializers/logentry.py create mode 100644 app/content/views/logentry.py create mode 100644 app/tests/content/test_logentry_integration.py diff --git a/app/content/admin/admin.py b/app/content/admin/admin.py index 92b9d8e6..8afc11f8 100644 --- a/app/content/admin/admin.py +++ b/app/content/admin/admin.py @@ -206,7 +206,8 @@ def has_delete_permission(self, request, obj=None): class LogEntryAdmin(admin.ModelAdmin): actions = None - date_hierarchy = "action_time" + # This breaks the admin panel becaause of the new DB is not configured + # date_hierarchy = "action_time" list_filter = ["user", "content_type", "action_flag"] diff --git a/app/content/factories/__init__.py b/app/content/factories/__init__.py index 780110ec..5c27302e 100644 --- a/app/content/factories/__init__.py +++ b/app/content/factories/__init__.py @@ -10,3 +10,4 @@ from app.content.factories.toddel_factory import ToddelFactory from app.content.factories.priority_pool_factory import PriorityPoolFactory from app.content.factories.qr_code_factory import QRCodeFactory +from app.content.factories.logentry_factory import LogEntryFactory diff --git a/app/content/factories/logentry_factory.py b/app/content/factories/logentry_factory.py new file mode 100644 index 00000000..18023d0f --- /dev/null +++ b/app/content/factories/logentry_factory.py @@ -0,0 +1,19 @@ +from django.contrib.admin.models import LogEntry +from django.utils import timezone + +import factory +from factory.django import DjangoModelFactory + +from app.content.factories.user_factory import UserFactory + + +class LogEntryFactory(DjangoModelFactory): + class Meta: + model = LogEntry + + action_time = timezone.now() + user = factory.SubFactory(UserFactory) + content_type = None + object_id = 1 + object_repr = "Test" + action_flag = 1 diff --git a/app/content/serializers/content_type.py b/app/content/serializers/content_type.py new file mode 100644 index 00000000..bd842276 --- /dev/null +++ b/app/content/serializers/content_type.py @@ -0,0 +1,15 @@ +from django.contrib.contenttypes.models import ContentType +from rest_framework import serializers + +from app.common.serializers import BaseModelSerializer + + +class ContentTypeSerializer(BaseModelSerializer): + app_label_name = serializers.SerializerMethodField() + + class Meta: + model = ContentType + fields = ("app_label_name",) + + def get_app_label_name(self, obj): + return obj.app_labeled_name diff --git a/app/content/serializers/logentry.py b/app/content/serializers/logentry.py new file mode 100644 index 00000000..9977272a --- /dev/null +++ b/app/content/serializers/logentry.py @@ -0,0 +1,31 @@ +from django.contrib.admin.models import LogEntry +from rest_framework import serializers + +from app.common.serializers import BaseModelSerializer +from app.content.serializers.content_type import ContentTypeSerializer +from app.content.serializers.user import SimpleUserSerializer + + +class LogEntryListSerializer(BaseModelSerializer): + user = SimpleUserSerializer(many=False) + content_type = ContentTypeSerializer(many=False) + action_flag = serializers.SerializerMethodField() + + class Meta: + model = LogEntry + fields = ( + "action_time", + "user", + "content_type", + "object_id", + "object_repr", + "action_flag", + ) + + def get_action_flag(self, obj): + if obj.is_addition(): + return "ADDITION" + if obj.is_change(): + return "CHANGE" + if obj.is_deletion(): + return "DELETION" diff --git a/app/content/urls.py b/app/content/urls.py index 3ac574c3..a1f51508 100644 --- a/app/content/urls.py +++ b/app/content/urls.py @@ -5,6 +5,7 @@ CategoryViewSet, CheatsheetViewSet, EventViewSet, + LogEntryViewSet, NewsViewSet, PageViewSet, QRCodeViewSet, @@ -40,6 +41,7 @@ ) router.register("pages", PageViewSet) router.register("strikes", StrikeViewSet, basename="strikes") +router.register("log-entries", LogEntryViewSet, basename="log-entries") urlpatterns = [ re_path(r"", include(router.urls)), diff --git a/app/content/views/__init__.py b/app/content/views/__init__.py index 2e48c38a..9a89d3ab 100644 --- a/app/content/views/__init__.py +++ b/app/content/views/__init__.py @@ -12,3 +12,4 @@ from app.content.views.strike import StrikeViewSet from app.content.views.toddel import ToddelViewSet from app.content.views.qr_code import QRCodeViewSet +from app.content.views.logentry import LogEntryViewSet diff --git a/app/content/views/logentry.py b/app/content/views/logentry.py new file mode 100644 index 00000000..4bc968c8 --- /dev/null +++ b/app/content/views/logentry.py @@ -0,0 +1,39 @@ +from django.contrib.admin.models import LogEntry +from rest_framework.response import Response + +from app.common.mixins import ActionMixin +from app.common.pagination import BasePagination +from app.common.permissions import AdminGroup, check_has_access +from app.common.viewsets import BaseViewSet +from app.content.serializers.logentry import LogEntryListSerializer + + +class LogEntryViewSet(BaseViewSet, ActionMixin): + serializer_class = LogEntryListSerializer + pagination_class = BasePagination + queryset = LogEntry.objects.all() + + def list(self, request, *args, **kwargs): + if check_has_access(AdminGroup.admin(), request): + return super().list(request, *args, **kwargs) + + return Response({"detail": "Du har ikke tilgang til å se loggen."}, status=403) + + def retrieve(self, request, *args, **kwargs): + if check_has_access(AdminGroup.admin(), request): + return super().retrieve(request, *args, **kwargs) + + return Response({"detail": "Du har ikke tilgang til å se loggen."}, status=403) + + def create(self, request, *args, **kwargs): + return Response({"detail": "Du har ikke tilgang til å logge."}, status=403) + + def update(self, request, *args, **kwargs): + return Response( + {"detail": "Du har ikke tilgang til å oppdatere loggen."}, status=403 + ) + + def destroy(self, request, *args, **kwargs): + return Response( + {"detail": "Du har ikke tilgang til å slette loggen."}, status=403 + ) diff --git a/app/tests/content/test_logentry_integration.py b/app/tests/content/test_logentry_integration.py new file mode 100644 index 00000000..0e9e6829 --- /dev/null +++ b/app/tests/content/test_logentry_integration.py @@ -0,0 +1,89 @@ +from rest_framework import status + +import pytest + +from app.content.factories import LogEntryFactory +from app.util.test_utils import get_api_client + +API_EVENTS_BASE_URL = "/log-entries/" + + +@pytest.mark.django_db +def test_logentry_list(admin_user): + """ + An admin should be able to list log entries. + """ + client = get_api_client(user=admin_user) + response = client.get(API_EVENTS_BASE_URL) + + assert response.status_code == status.HTTP_200_OK + + +@pytest.mark.django_db +def test_logentry_list_no_access(user): + """ + An user should not be able to list log entries. + """ + client = get_api_client(user=user) + response = client.get(API_EVENTS_BASE_URL) + + assert response.status_code == status.HTTP_403_FORBIDDEN + + +@pytest.mark.django_db +def test_logentry_retrieve(admin_user): + """ + An admin should be able to retrieve a log entry. + """ + log = LogEntryFactory() + client = get_api_client(user=admin_user) + response = client.get(f"{API_EVENTS_BASE_URL}{log.id}/") + + assert response.status_code == status.HTTP_200_OK + + +@pytest.mark.django_db +def test_logentry_retrieve_no_access(user): + """ + An user should not be able to retrieve a log entry. + """ + log = LogEntryFactory() + client = get_api_client(user=user) + response = client.get(f"{API_EVENTS_BASE_URL}{log.id}/") + + assert response.status_code == status.HTTP_403_FORBIDDEN + + +@pytest.mark.django_db +def test_logentry_create(admin_user): + """ + An admin should not be able to create a log entry. + """ + client = get_api_client(user=admin_user) + response = client.post(API_EVENTS_BASE_URL, data={}) + + assert response.status_code == status.HTTP_403_FORBIDDEN + + +@pytest.mark.django_db +def test_logentry_update(admin_user): + """ + An admin should not be able to update a log entry. + """ + log = LogEntryFactory() + client = get_api_client(user=admin_user) + response = client.put(f"{API_EVENTS_BASE_URL}{log.id}/", data={}) + + assert response.status_code == status.HTTP_403_FORBIDDEN + + +@pytest.mark.django_db +def test_logentry_destroy(admin_user): + """ + An admin should not be able to destroy a log entry. + """ + log = LogEntryFactory() + client = get_api_client(user=admin_user) + response = client.delete(f"{API_EVENTS_BASE_URL}{log.id}/") + + assert response.status_code == status.HTTP_403_FORBIDDEN From dfdbae3850d92b695c8b2891833333bb6ae263f9 Mon Sep 17 00:00:00 2001 From: Erik Skjellevik <98759397+eriskjel@users.noreply.github.com> Date: Wed, 21 Feb 2024 21:38:32 +0100 Subject: [PATCH 11/23] Feat(kontres)/add group to create reservation (#769) * fixed delete object permission * added group field to reservation model * added context to serializer * added group validation logic to serializer * created tests for group logic on reservation model * fixed seralizer complexity by splitting into methods * fixed time issue on tests * linting * fixed bug where bookable item was not properly inserted as payload in 2 tests * removed unnecessary assertion against database * fixed destroy logic in viewset and model --- app/kontres/factories/reservation_factory.py | 4 +- .../migrations/0004_reservation_group.py | 26 +++ app/kontres/models/reservation.py | 13 ++ .../serializer/reservation_seralizer.py | 56 +++-- app/kontres/views/reservation.py | 8 +- .../kontres/test_reservation_integration.py | 194 ++++++++++++++++-- app/tests/kontres/test_reservation_model.py | 14 ++ 7 files changed, 277 insertions(+), 38 deletions(-) create mode 100644 app/kontres/migrations/0004_reservation_group.py diff --git a/app/kontres/factories/reservation_factory.py b/app/kontres/factories/reservation_factory.py index 791289b2..4415f071 100644 --- a/app/kontres/factories/reservation_factory.py +++ b/app/kontres/factories/reservation_factory.py @@ -15,7 +15,7 @@ class Meta: author = SubFactory(UserFactory) bookable_item = SubFactory(BookableItemFactory) - start_time = timezone.now() - end_time = timezone.now() + timezone.timedelta(hours=1) + start_time = timezone.now() + timezone.timedelta(hours=1) + end_time = timezone.now() + timezone.timedelta(hours=2) state = ReservationStateEnum.PENDING description = Faker("text") diff --git a/app/kontres/migrations/0004_reservation_group.py b/app/kontres/migrations/0004_reservation_group.py new file mode 100644 index 00000000..ba8e3f58 --- /dev/null +++ b/app/kontres/migrations/0004_reservation_group.py @@ -0,0 +1,26 @@ +# Generated by Django 4.2.5 on 2024-02-21 19:02 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ("group", "0018_fine_defense"), + ("kontres", "0003_reservation_description"), + ] + + operations = [ + migrations.AddField( + model_name="reservation", + name="group", + field=models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name="reservations", + to="group.group", + ), + ), + ] diff --git a/app/kontres/models/reservation.py b/app/kontres/models/reservation.py index 9d205261..1a7260ba 100644 --- a/app/kontres/models/reservation.py +++ b/app/kontres/models/reservation.py @@ -5,6 +5,7 @@ from app.common.enums import AdminGroup, Groups from app.common.permissions import BasePermissionModel, check_has_access from app.content.models import User +from app.group.models import Group from app.kontres.enums import ReservationStateEnum from app.kontres.models.bookable_item import BookableItem from app.util.models import BaseModel @@ -29,6 +30,13 @@ class Reservation(BaseModel, BasePermissionModel): ) description = models.TextField(blank=True) accepted_rules = models.BooleanField(default=True) + group = models.ForeignKey( + Group, + on_delete=models.SET_NULL, + related_name="reservations", + null=True, + blank=True, + ) def __str__(self): return f"{self.state} - Reservation request by {self.author.first_name} {self.author.last_name} to book {self.bookable_item.name}. Created at {self.created_at}" @@ -49,6 +57,11 @@ def has_update_permission(cls, request): def has_destroy_permission(cls, request): return check_has_access(cls.write_access, request) + def has_object_destroy_permission(self, request): + is_owner = self.author == request.user + is_admin = check_has_access([AdminGroup.INDEX, AdminGroup.HS], request) + return is_owner or is_admin + @classmethod def has_create_permission(cls, request): return check_has_access(cls.write_access, request) diff --git a/app/kontres/serializer/reservation_seralizer.py b/app/kontres/serializer/reservation_seralizer.py index 7cb8ac9e..cc97b6a7 100644 --- a/app/kontres/serializer/reservation_seralizer.py +++ b/app/kontres/serializer/reservation_seralizer.py @@ -2,6 +2,8 @@ from django.utils import timezone from rest_framework import serializers +from app.group.models import Group +from app.kontres.enums import ReservationStateEnum from app.kontres.models.bookable_item import BookableItem from app.kontres.models.reservation import Reservation @@ -10,44 +12,73 @@ class ReservationSerializer(serializers.ModelSerializer): bookable_item = serializers.PrimaryKeyRelatedField( queryset=BookableItem.objects.all() ) + group = serializers.SlugRelatedField( + slug_field="slug", queryset=Group.objects.all(), required=False, allow_null=True + ) class Meta: model = Reservation fields = "__all__" - def validate_start_time(self, start_time): - if start_time < timezone.now(): - raise serializers.ValidationError("Start-tiden kan ikke være i fortiden.") - return start_time - def validate(self, data): + user = self.context["request"].user + group = data.get("group", None) + if group: + self.validate_group(group) + self.validate_state_change(data, user) + self.validate_time_and_overlapping(data) + return data - # Extract the bookable_item, start_time, and end_time, accounting for the possibility they may not be provided - bookable_item = data.get( - "bookable_item", self.instance.bookable_item if self.instance else None - ) + def validate_group(self, value): + user = self.context["request"].user + group = value + + if self.instance and group != self.instance.group: + # Assuming your model logic and permissions are correctly implemented in is_HS_or_Index_member + if ( + not user.is_HS_or_Index_member + and self.instance.state != ReservationStateEnum.PENDING + ): + raise serializers.ValidationError( + "Du har ikke tilgang til å endre gruppen til denne reservasjonsforespørselen." + ) + if group and not user.is_member_of(group): + raise serializers.ValidationError( + f"Du er ikke medlem av {group.slug} og kan dermed ikke legge inn bestilling på deres vegne." + ) + + return group + + def validate_state_change(self, data, user): # Validate the state change permission if "state" in data: if self.instance and data["state"] != self.instance.state: - user = self.context["request"].user if not (user and user.is_authenticated and user.is_HS_or_Index_member): raise serializers.ValidationError( { "state": "Du har ikke rettigheter til å endre reservasjonsstatusen." } ) + pass + + def validate_time_and_overlapping(self, data): # Validate that the end time is after the start time start_time = data.get( "start_time", self.instance.start_time if self.instance else None ) + if start_time < timezone.now(): + raise serializers.ValidationError("Start-tiden kan ikke være i fortiden.") end_time = data.get( "end_time", self.instance.end_time if self.instance else None ) if start_time and end_time and end_time <= start_time: raise serializers.ValidationError("Slutt-tid må være etter start-tid") - + # Extract the bookable_item, start_time, and end_time, accounting for the possibility they may not be provided + bookable_item = data.get( + "bookable_item", self.instance.bookable_item if self.instance else None + ) # Check for overlapping reservations only if necessary fields are present if bookable_item and start_time and end_time: # Build the query for overlapping reservations @@ -64,5 +95,4 @@ def validate(self, data): raise serializers.ValidationError( "Det er en reservasjonsoverlapp for det gitte tidsrommet." ) - - return data + pass diff --git a/app/kontres/views/reservation.py b/app/kontres/views/reservation.py index f48cd435..a974a25b 100644 --- a/app/kontres/views/reservation.py +++ b/app/kontres/views/reservation.py @@ -34,7 +34,9 @@ def get_queryset(self): def create(self, request, *args, **kwargs): request.data["author"] = request.user.user_id - serializer = ReservationSerializer(data=request.data) + serializer = ReservationSerializer( + data=request.data, context={"request": request} + ) if serializer.is_valid(): # Overriding the state to PENDING serializer.validated_data["state"] = "PENDING" @@ -48,3 +50,7 @@ def update(self, request, *args, **kwargs): serializer.is_valid(raise_exception=True) serializer.save() return Response(serializer.data, status=status.HTTP_200_OK) + + def destroy(self, request, *args, **kwargs): + super().destroy(self, request, *args, **kwargs) + return Response(status=status.HTTP_204_NO_CONTENT) diff --git a/app/tests/kontres/test_reservation_integration.py b/app/tests/kontres/test_reservation_integration.py index 83a20e05..c9868f87 100644 --- a/app/tests/kontres/test_reservation_integration.py +++ b/app/tests/kontres/test_reservation_integration.py @@ -6,11 +6,13 @@ import pytest from app.common.enums import AdminGroup +from app.group.factories import GroupFactory from app.kontres.enums import ReservationStateEnum from app.kontres.factories.bookable_item_factory import BookableItemFactory from app.kontres.factories.reservation_factory import ReservationFactory from app.kontres.models.bookable_item import BookableItem from app.kontres.models.reservation import Reservation +from app.tests.conftest import _add_user_to_group from app.util.test_utils import get_api_client @@ -71,8 +73,8 @@ def test_non_tihlde_cannot_create_reservation(user, bookable_item): { "author": user.user_id, "bookable_item": bookable_item.id, - "start_time": "2023-10-10T10:00:00Z", - "end_time": "2023-10-10T11:00:00Z", + "start_time": "2025-10-10T10:00:00Z", + "end_time": "2025-10-10T11:00:00Z", }, format="json", ) @@ -99,11 +101,28 @@ def test_creating_reservation_with_past_start_time(member, bookable_item): @pytest.mark.django_db def test_member_deleting_own_reservation(member, reservation): + reservation.author = member + 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 +@pytest.mark.django_db +def test_member_cannot_update_random_reservation(member, reservation): + client = get_api_client(user=member) + + new_description = "Updated description" + response = client.patch( + f"/kontres/reservations/{reservation.id}/", + {"description": new_description}, + format="json", + ) + + assert response.status_code == 403 + assert "description" not in response.data + + @pytest.mark.django_db def test_user_cannot_create_confirmed_reservation(bookable_item, member): client = get_api_client(user=member) @@ -129,22 +148,6 @@ def test_user_cannot_create_confirmed_reservation(bookable_item, member): assert response.data["state"] == "PENDING" -@pytest.mark.django_db -def test_user_cannot_create_reservation_without_author(member, bookable_item): - client = get_api_client(user=member) - response = client.post( - "/kontres/reservations/", - { - "bookable_item_id": bookable_item.id, - "start_time": "2023-10-10T10:00:00Z", - "end_time": "2023-10-10T11:00:00Z", - }, - format="json", - ) - - assert response.status_code == 400 - - @pytest.mark.django_db def test_user_cannot_create_reservation_with_invalid_date_format(member, bookable_item): client = get_api_client(user=member) @@ -152,7 +155,7 @@ def test_user_cannot_create_reservation_with_invalid_date_format(member, bookabl "/kontres/reservations/", { "author": member.user_id, - "bookable_item_id": bookable_item.id, + "bookable_item": bookable_item.id, "start_time": "invalid_date_format", "end_time": "2023-10-10T11:00:00Z", }, @@ -373,7 +376,7 @@ def test_user_cannot_create_reservation_with_end_time_before_start_time( { "author": member.user_id, "bookable_item": bookable_item.id, - "start_time": "2023-10-10T12:00:00Z", + "start_time": "2025-10-10T12:00:00Z", "end_time": "2023-10-10T11:00:00Z", }, format="json", @@ -404,8 +407,8 @@ def test_unauthenticated_request_cannot_create_reservation(bookable_item): "/kontres/reservations/", { "bookable_item": bookable_item.id, - "start_time": "2023-10-10T10:00:00Z", - "end_time": "2023-10-10T11:00:00Z", + "start_time": "2025-10-10T10:00:00Z", + "end_time": "2025-10-10T11:00:00Z", }, format="json", ) @@ -598,3 +601,150 @@ def test_admin_can_update_details_of_confirmed_reservation(admin_user, reservati assert response.status_code == 200 assert response.data["description"] == new_description + + +@pytest.mark.django_db +def test_user_can_change_reservation_group(member, reservation): + # Setup: Create two groups and add the user to both + original_group = GroupFactory() + new_group = GroupFactory() + _add_user_to_group(member, original_group) + _add_user_to_group(member, new_group) + + # Assign the original group to the reservation and save + reservation.group = original_group + reservation.author = member + reservation.save() + + # Prepare the client and attempt to update the reservation's group + client = get_api_client(user=member) + reservation_id = str(reservation.id) + response = client.put( + f"/kontres/reservations/{reservation_id}/", + { + "group": new_group.slug, + }, + format="json", + ) + + # Verify the response + assert response.status_code == status.HTTP_200_OK, response.data + assert ( + response.data["group"] == new_group.slug + ), "Group should be updated to the new group" + + +@pytest.mark.django_db +def test_user_can_create_reservation_for_group(member, bookable_item, group): + client = get_api_client(user=member) + + _add_user_to_group(member, group) + + response = client.post( + "/kontres/reservations/", + { + "group": group.slug, + "bookable_item": bookable_item.id, + "start_time": "2030-10-10T10:00:00Z", + "end_time": "2030-10-10T11:00:00Z", + }, + format="json", + ) + + assert response.status_code == 201 + + +@pytest.mark.django_db +def test_user_cannot_create_reservation_for_group_if_not_member_of_group( + member, bookable_item, group +): + client = get_api_client(user=member) + + response = client.post( + "/kontres/reservations/", + { + "group": group.slug, + "bookable_item": bookable_item.id, + "start_time": "2030-10-10T10:00:00Z", + "end_time": "2030-10-10T11:00:00Z", + }, + format="json", + ) + + assert response.status_code == 400 + + +@pytest.mark.django_db +def test_user_cannot_create_reservation_for_another_group(member, bookable_item): + client = get_api_client(user=member) + + group1 = GroupFactory() + group2 = GroupFactory() + + _add_user_to_group(member, group1) + + response = client.post( + "/kontres/reservations/", + { + "group": group2.slug, + "bookable_item": bookable_item.id, + "start_time": "2030-10-10T10:00:00Z", + "end_time": "2030-10-10T11:00:00Z", + }, + format="json", + ) + + assert response.status_code == 400 + + +@pytest.mark.django_db +def test_user_can_change_reservation_group_if_state_is_pending(member, reservation): + original_group = GroupFactory() + new_group = GroupFactory() + _add_user_to_group(member, original_group) + _add_user_to_group(member, new_group) + + reservation.group = original_group + reservation.author = member + reservation.save() + + client = get_api_client(user=member) + reservation_id = str(reservation.id) + response = client.put( + f"/kontres/reservations/{reservation_id}/", + { + "group": new_group.slug, + }, + format="json", + ) + + # Verify the response + assert response.status_code == status.HTTP_200_OK, response.data + assert response.data["group"] == new_group.slug + + +@pytest.mark.django_db +def test_user_cannot_change_reservation_group_if_state_is_not_pending( + member, reservation +): + original_group = GroupFactory() + new_group = GroupFactory() + _add_user_to_group(member, original_group) + _add_user_to_group(member, new_group) + + reservation.group = original_group + reservation.author = member + reservation.state = ReservationStateEnum.CONFIRMED + reservation.save() + + client = get_api_client(user=member) + reservation_id = str(reservation.id) + response = client.put( + f"/kontres/reservations/{reservation_id}/", + { + "group": new_group.slug, + }, + format="json", + ) + + assert response.status_code == status.HTTP_400_BAD_REQUEST diff --git a/app/tests/kontres/test_reservation_model.py b/app/tests/kontres/test_reservation_model.py index aff4e80f..cf9d8ecb 100644 --- a/app/tests/kontres/test_reservation_model.py +++ b/app/tests/kontres/test_reservation_model.py @@ -113,3 +113,17 @@ def test_reservation_requires_author(): end_time=timezone.now() + timezone.timedelta(hours=1), bookable_item=bookable_item, ) + + +@pytest.mark.django_db +def test_reservation_with_group(group): + user = User.objects.create(user_id="test_user") + bookable_item = BookableItem.objects.create(name="Test Item") + reservation = Reservation.objects.create( + author=user, + bookable_item=bookable_item, + start_time=timezone.now(), + end_time=timezone.now() + timezone.timedelta(hours=1), + group=group, + ) + assert reservation.group == group From 8f9aeceb566885bc30017168e40f20ed0470c940 Mon Sep 17 00:00:00 2001 From: Erik Skjellevik <98759397+eriskjel@users.noreply.github.com> Date: Thu, 22 Feb 2024 19:01:51 +0100 Subject: [PATCH 12/23] Feat(kontres)/return full objects in response (#770) * reservation response will now include the full objects for author, bookable_item and group * reservation response will now include the full objects for author, bookable_item and group * git aids * fixed some more git aids --- .../serializer/reservation_seralizer.py | 21 +++++++++-- app/kontres/views/reservation.py | 9 ++--- .../kontres/test_reservation_integration.py | 36 ++++++++++--------- 3 files changed, 42 insertions(+), 24 deletions(-) diff --git a/app/kontres/serializer/reservation_seralizer.py b/app/kontres/serializer/reservation_seralizer.py index cc97b6a7..6fdfda98 100644 --- a/app/kontres/serializer/reservation_seralizer.py +++ b/app/kontres/serializer/reservation_seralizer.py @@ -2,19 +2,34 @@ from django.utils import timezone from rest_framework import serializers +from app.content.models import User +from app.content.serializers import UserSerializer from app.group.models import Group +from app.group.serializers import GroupSerializer from app.kontres.enums import ReservationStateEnum from app.kontres.models.bookable_item import BookableItem from app.kontres.models.reservation import Reservation +from app.kontres.serializer.bookable_item_serializer import ( + BookableItemSerializer, +) class ReservationSerializer(serializers.ModelSerializer): bookable_item = serializers.PrimaryKeyRelatedField( - queryset=BookableItem.objects.all() + queryset=BookableItem.objects.all(), write_only=True, required=False ) - group = serializers.SlugRelatedField( - slug_field="slug", queryset=Group.objects.all(), required=False, allow_null=True + bookable_item_detail = BookableItemSerializer( + source="bookable_item", read_only=True ) + group = serializers.PrimaryKeyRelatedField( + queryset=Group.objects.all(), write_only=True, required=False + ) + group_detail = GroupSerializer(source="group", read_only=True) + + author = serializers.PrimaryKeyRelatedField( + queryset=User.objects.all(), write_only=True, required=False + ) + author_detail = UserSerializer(source="author", read_only=True) class Meta: model = Reservation diff --git a/app/kontres/views/reservation.py b/app/kontres/views/reservation.py index a974a25b..eeeb8721 100644 --- a/app/kontres/views/reservation.py +++ b/app/kontres/views/reservation.py @@ -5,6 +5,7 @@ from app.common.permissions import BasicViewPermission from app.common.viewsets import BaseViewSet +from app.kontres.enums import ReservationStateEnum from app.kontres.models.reservation import Reservation from app.kontres.serializer.reservation_seralizer import ReservationSerializer @@ -33,16 +34,16 @@ def get_queryset(self): return Reservation.objects.all() def create(self, request, *args, **kwargs): - request.data["author"] = request.user.user_id serializer = ReservationSerializer( data=request.data, context={"request": request} ) if serializer.is_valid(): - # Overriding the state to PENDING - serializer.validated_data["state"] = "PENDING" + serializer.validated_data["author"] = request.user + serializer.validated_data["state"] = ReservationStateEnum.PENDING serializer.save() return Response(serializer.data, status=status.HTTP_201_CREATED) - return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) + else: + return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) def update(self, request, *args, **kwargs): reservation = self.get_object() diff --git a/app/tests/kontres/test_reservation_integration.py b/app/tests/kontres/test_reservation_integration.py index c9868f87..9a1f0c0f 100644 --- a/app/tests/kontres/test_reservation_integration.py +++ b/app/tests/kontres/test_reservation_integration.py @@ -31,20 +31,22 @@ def test_member_can_create_reservation(member, bookable_item): ) assert response.status_code == 201 - assert response.data["author"] == member.user_id - assert response.data["bookable_item"] == bookable_item.id + assert response.data["author_detail"]["user_id"] == str(member.user_id) + assert response.data["bookable_item_detail"]["id"] == str(bookable_item.id) assert response.data["state"] == "PENDING" @pytest.mark.django_db -def test_member_cannot_set_different_author_in_reservation(member, bookable_item): +def test_member_cannot_set_different_author_in_reservation( + member, bookable_item, sosialen_user +): client = get_api_client(user=member) # Attempt to create a reservation with a different author specified in the request body response = client.post( "/kontres/reservations/", { - "author": "different_user_id", # Attempt to set a different author + "author": sosialen_user.user_id, "bookable_item": bookable_item.id, "start_time": "2030-10-10T10:00:00Z", "end_time": "2030-10-10T11:00:00Z", @@ -56,11 +58,11 @@ def test_member_cannot_set_different_author_in_reservation(member, bookable_item assert response.status_code == 201 # Check that the author of the reservation is actually the requesting user - assert response.data["author"] == member.user_id - assert response.data["author"] != "different_user_id" + assert response.data["author_detail"]["user_id"] == member.user_id + assert response.data["author_detail"]["user_id"] != "different_user_id" # Check other attributes of the reservation - assert response.data["bookable_item"] == bookable_item.id + assert response.data["bookable_item_detail"]["id"] == str(bookable_item.id) assert response.data["state"] == "PENDING" @@ -137,7 +139,6 @@ def test_user_cannot_create_confirmed_reservation(bookable_item, member): { "author": member.user_id, "bookable_item": bookable_item.id, - # Format start_time and end_time to ISO format for the POST request "start_time": start_time.isoformat(), "end_time": end_time.isoformat(), "state": "CONFIRMED", @@ -251,8 +252,12 @@ def test_user_can_fetch_all_reservations(reservation, member): first_reservation = Reservation.objects.first() assert str(response.data[0]["id"]) == str(first_reservation.id) - assert response.data[0]["author"] == first_reservation.author.user_id - assert response.data[0]["bookable_item"] == first_reservation.bookable_item.id + assert ( + response.data[0]["author_detail"]["user_id"] == first_reservation.author.user_id + ) + assert response.data[0]["bookable_item_detail"]["id"] == str( + first_reservation.bookable_item.id + ) assert response.data[0]["state"] == "PENDING" @@ -290,8 +295,8 @@ def test_can_fetch_single_reservation(reservation, member): assert response.status_code == 200 assert str(response.data["id"]) == str(reservation.id) - assert response.data["author"] == reservation.author.user_id - assert str(response.data["bookable_item"]) == str( + assert response.data["author_detail"]["user_id"] == reservation.author.user_id + assert str(response.data["bookable_item_detail"]["id"]) == str( reservation.bookable_item.id ) # Convert both to string assert response.data["state"] == "PENDING" @@ -627,11 +632,8 @@ def test_user_can_change_reservation_group(member, reservation): format="json", ) - # Verify the response assert response.status_code == status.HTTP_200_OK, response.data - assert ( - response.data["group"] == new_group.slug - ), "Group should be updated to the new group" + assert response.data["group_detail"]["slug"] == new_group.slug @pytest.mark.django_db @@ -720,7 +722,7 @@ def test_user_can_change_reservation_group_if_state_is_pending(member, reservati # Verify the response assert response.status_code == status.HTTP_200_OK, response.data - assert response.data["group"] == new_group.slug + assert response.data["group_detail"]["slug"] == new_group.slug @pytest.mark.django_db From 25398299df6dfb2e1a5ffb4c145179f6eb052e79 Mon Sep 17 00:00:00 2001 From: Erik Skjellevik <98759397+eriskjel@users.noreply.github.com> Date: Sat, 24 Feb 2024 14:19:45 +0100 Subject: [PATCH 13/23] fixed logic in serializer method for validating time (#771) * fixed logic in serializer method for validating time * skip unfinished test - waiting for updated viewset logic --- .../serializer/reservation_seralizer.py | 18 +++++++++--- .../kontres/test_reservation_integration.py | 28 +++++++++++++------ 2 files changed, 34 insertions(+), 12 deletions(-) diff --git a/app/kontres/serializer/reservation_seralizer.py b/app/kontres/serializer/reservation_seralizer.py index 6fdfda98..5be0b977 100644 --- a/app/kontres/serializer/reservation_seralizer.py +++ b/app/kontres/serializer/reservation_seralizer.py @@ -79,18 +79,28 @@ def validate_state_change(self, data, user): def validate_time_and_overlapping(self, data): - # Validate that the end time is after the start time + # Check if this is an update operation and if start_time is being modified. + is_update_operation = self.instance is not None + start_time_being_modified = "start_time" in data + + # Retrieve the start and end times from the data if provided, else from the instance. start_time = data.get( "start_time", self.instance.start_time if self.instance else None ) - if start_time < timezone.now(): - raise serializers.ValidationError("Start-tiden kan ikke være i fortiden.") end_time = data.get( "end_time", self.instance.end_time if self.instance else None ) + + # Skip the past start time check if this is an update and the start time isn't being modified. + if not (is_update_operation and not start_time_being_modified): + if start_time < timezone.now(): + raise serializers.ValidationError( + "Start-tiden kan ikke være i fortiden." + ) + + # Ensure the end time is after the start time for all operations. if start_time and end_time and end_time <= start_time: raise serializers.ValidationError("Slutt-tid må være etter start-tid") - # Extract the bookable_item, start_time, and end_time, accounting for the possibility they may not be provided bookable_item = data.get( "bookable_item", self.instance.bookable_item if self.instance else None ) diff --git a/app/tests/kontres/test_reservation_integration.py b/app/tests/kontres/test_reservation_integration.py index 9a1f0c0f..69c0a559 100644 --- a/app/tests/kontres/test_reservation_integration.py +++ b/app/tests/kontres/test_reservation_integration.py @@ -170,6 +170,8 @@ def test_user_cannot_create_reservation_with_invalid_date_format(member, bookabl def test_admin_can_edit_reservation_to_confirmed(reservation, admin_user): client = get_api_client(user=admin_user) + assert reservation.state == ReservationStateEnum.PENDING + response = client.put( f"/kontres/reservations/{reservation.id}/", {"state": "CONFIRMED"}, @@ -177,7 +179,7 @@ def test_admin_can_edit_reservation_to_confirmed(reservation, admin_user): ) assert response.status_code == 200 - assert response.data["state"] == "CONFIRMED" + assert response.data["state"] == ReservationStateEnum.CONFIRMED @pytest.mark.django_db @@ -480,6 +482,7 @@ def test_retrieve_specific_reservation_within_its_date_range(member, bookable_it assert any(res["id"] == str(reservation.id) for res in response.data) +@pytest.mark.skip @pytest.mark.django_db def test_retrieve_subset_of_reservations(member, bookable_item): client = get_api_client(user=member) @@ -519,13 +522,22 @@ def test_retrieve_subset_of_reservations(member, bookable_item): format="json", ) - # Define the query date range to include only the first two reservations - query_start_date = current_time.replace( - hour=9, minute=0, second=0, microsecond=0 - ).isoformat() - query_end_date = current_time.replace( - hour=9, minute=0, second=0, microsecond=0, day=current_time.day + 2 - ).isoformat() + from django.utils.timezone import get_current_timezone + + # Example of formatting the datetime with timezone information + query_start_date = ( + current_time.replace(hour=9, minute=0, second=0, microsecond=0) + .astimezone(get_current_timezone()) + .isoformat() + ) + + query_end_date = ( + current_time.replace( + hour=9, minute=0, second=0, microsecond=0, day=current_time.day + 1 + ) + .astimezone(get_current_timezone()) + .isoformat() + ) # Retrieve reservations for the specific date range response = client.get( From 0d69812c80b4b56fcc22a802f3c0b1bbb4f4f151 Mon Sep 17 00:00:00 2001 From: Mads Nylund <73914541+MadsNyl@users.noreply.github.com> Date: Mon, 4 Mar 2024 20:32:27 +0100 Subject: [PATCH 14/23] Update pull_request_template.md (#774) --- .github/pull_request_template.md | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 547f276f..b8a9767b 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -1,15 +1,14 @@ ## Proposed changes -Describe the big picture of your changes here to communicate to the maintainers why we should accept this pull request. If it fixes a bug or resolves a feature request, be sure to link to that issue. +Describe the big picture of your changes here to communicate to the maintainers why we should accept this pull request. If it fixes a bug or resolves a feature request, be sure to link to that issue. Remove this part with the description. -Issue number: closes # +Issue number: closes # (remove if not an issue) ## Pull request checklist Please check if your PR fulfills the following requirements: -- [ ] CHANGELOG.md has been updated. [Guide](https://tihlde.slab.com/posts/changelog-z8hybjom) - [ ] Tests for the changes have been added (for bug fixes / features) -- [ ] Docs have been reviewed and added / updated if needed (for bug fixes / features) +- [ ] API docs on [Codex](https://codex.tihlde.org/contributing) have been reviewed and added / updated if needed (for bug fixes / features) - [ ] The fixtures have been updated if needed (for migrations) ## Further comments From d81ade323fd7cf3dc4cecef01de05fee54c40ed4 Mon Sep 17 00:00:00 2001 From: Erik Skjellevik <98759397+eriskjel@users.noreply.github.com> Date: Wed, 6 Mar 2024 08:06:03 +0100 Subject: [PATCH 15/23] Fix(event)/fix priority waiting number (#775) --- app/content/models/registration.py | 45 +++++--- app/tests/content/test_event_integration.py | 108 ++++++++++++++++++++ 2 files changed, 141 insertions(+), 12 deletions(-) diff --git a/app/content/models/registration.py b/app/content/models/registration.py index 1ccda13c..4cb84d5e 100644 --- a/app/content/models/registration.py +++ b/app/content/models/registration.py @@ -294,20 +294,41 @@ def is_prioritized(self): @property def wait_queue_number(self): - """ - Returns the number of people in front of the user in the waiting list. - """ - waiting_list_count = ( - self.event.get_waiting_list() - .order_by("-created_at") - .filter(created_at__lte=self.created_at) - .count() - ) - - if waiting_list_count == 0 or not self.is_on_wait: + # Return None if the user is not on the waitlist to indicate they are not waiting for a spot. + if not self.is_on_wait: return None - return waiting_list_count + # Retrieve all registrations for the event that are on the waitlist and order them by creation time. + waiting_list_registrations = self.event.registrations.filter( + is_on_wait=True + ).order_by("created_at") + + # Separate the waiting list registrations into prioritized and non-prioritized groups. + prioritized_registrations = [ + reg for reg in waiting_list_registrations if reg.is_prioritized + ] + non_prioritized_registrations = [ + reg for reg in waiting_list_registrations if not reg.is_prioritized + ] + + # If the registration is prioritized, calculate its queue position among other prioritized registrations. + if self.is_prioritized: + if self in prioritized_registrations: + queue_position = prioritized_registrations.index(self) + 1 + else: + return None + else: + # For non-prioritized registrations, calculate queue position considering all prioritized registrations first. + if self in non_prioritized_registrations: + queue_position = ( + len(prioritized_registrations) + + non_prioritized_registrations.index(self) + + 1 + ) + else: + return None + + return queue_position def swap_users(self): """Swaps a user with a spot with a prioritized user, if such user exists""" diff --git a/app/tests/content/test_event_integration.py b/app/tests/content/test_event_integration.py index 12c508bf..a4b4d3f2 100644 --- a/app/tests/content/test_event_integration.py +++ b/app/tests/content/test_event_integration.py @@ -12,6 +12,7 @@ from app.forms.tests.form_factories import EventFormFactory from app.group.factories import GroupFactory from app.group.models import Group +from app.tests.conftest import _add_user_to_group from app.util import now from app.util.test_utils import ( add_user_to_group_with_name, @@ -978,3 +979,110 @@ def test_create_paid_event(api_client, admin_user): assert data["is_paid_event"] assert data["paid_information"]["price"] == "200.00" assert data["paid_information"]["paytime"] == "01:00:00" + + +@pytest.mark.django_db +def test_wait_queue_number_for_prioritized_registration( + event_with_priority_pool, user_in_priority_pool, member, priority_group +): + prioritized_user_1 = UserFactory() + _add_user_to_group(prioritized_user_1, priority_group) + + prioritized_user_2 = UserFactory() + _add_user_to_group(prioritized_user_2, priority_group) + + prioritized_user_3 = UserFactory() + _add_user_to_group(prioritized_user_3, priority_group) + + RegistrationFactory( + event=event_with_priority_pool, user=prioritized_user_1, is_on_wait=True + ) + second_prioritized_registration = RegistrationFactory( + event=event_with_priority_pool, user=prioritized_user_2, is_on_wait=True + ) + third_prioritized_registration = RegistrationFactory( + event=event_with_priority_pool, user=prioritized_user_3, is_on_wait=True + ) + + assert second_prioritized_registration.wait_queue_number == 1 + assert third_prioritized_registration.wait_queue_number == 2 + + +@pytest.mark.django_db +def test_wait_queue_number_respects_priority_pools( + event_with_priority_pool, user_in_priority_pool, member, priority_group +): + prioritized_user_0 = UserFactory() + _add_user_to_group(prioritized_user_0, priority_group) + RegistrationFactory( + event=event_with_priority_pool, user=prioritized_user_0, is_on_wait=False + ) + + non_prioritized_registration = RegistrationFactory( + event=event_with_priority_pool, user=member, is_on_wait=True + ) + + prioritized_user_2 = UserFactory() + _add_user_to_group(prioritized_user_2, priority_group) + prioritized_user_3 = UserFactory() + _add_user_to_group(prioritized_user_3, priority_group) + + second_prioritized_registration = RegistrationFactory( + event=event_with_priority_pool, user=prioritized_user_2, is_on_wait=True + ) + third_prioritized_registration = RegistrationFactory( + event=event_with_priority_pool, user=prioritized_user_3, is_on_wait=True + ) + + assert second_prioritized_registration.wait_queue_number == 1 + assert third_prioritized_registration.wait_queue_number == 2 + assert non_prioritized_registration.wait_queue_number == 3 + + +@pytest.mark.django_db +def test_prioritized_users_always_ahead_of_non_prioritized( + event_with_priority_pool, priority_group +): + non_prioritized_users = [ + UserFactory() for _ in range(2) + ] # Create 2 non-prioritized users + prioritized_users = [UserFactory() for _ in range(2)] # Create 2 prioritized users + + # simulate the event being filled + prioritized_user_0 = UserFactory() + _add_user_to_group(prioritized_user_0, priority_group) + RegistrationFactory( + event=event_with_priority_pool, user=prioritized_user_0, is_on_wait=False + ) + + # Assign users to priority group and register them + for user in prioritized_users: + _add_user_to_group(user, priority_group) + + # Non-prioritized users register first and are placed on the waitlist + for user in non_prioritized_users: + RegistrationFactory(event=event_with_priority_pool, user=user, is_on_wait=True) + + # Prioritized users register after and are also placed on the waitlist + for user in prioritized_users: + RegistrationFactory(event=event_with_priority_pool, user=user, is_on_wait=True) + + # Fetch registrations that are specifically on the waitlist and prioritize accordingly + waitlist_registrations = event_with_priority_pool.registrations.filter( + is_on_wait=True + ).order_by("created_at") + + # Extract wait queue numbers for prioritized and non-prioritized users on the waitlist + prioritized_wait_numbers = [ + reg.wait_queue_number for reg in waitlist_registrations if reg.is_prioritized + ] + non_prioritized_wait_numbers = [ + reg.wait_queue_number + for reg in waitlist_registrations + if not reg.is_prioritized + ] + + # Ensure all prioritized users have lower wait queue numbers than any non-prioritized user + assert all( + p_num < min(non_prioritized_wait_numbers) for p_num in prioritized_wait_numbers + ), "Prioritized users do not all precede non-prioritized users in the wait queue" From b3d3456a018b1987aa01a3e9b4a0c4b33a38ddb4 Mon Sep 17 00:00:00 2001 From: Erik Skjellevik <98759397+eriskjel@users.noreply.github.com> Date: Wed, 6 Mar 2024 08:11:59 +0100 Subject: [PATCH 16/23] fix(kontres)/added basic viewset to bookable item (#773) --- ...05_bookableitem_allows_alcohol_and_more.py | 57 +++++++++++++ app/kontres/models/bookable_item.py | 10 +++ app/kontres/models/reservation.py | 20 ++++- .../serializer/reservation_seralizer.py | 31 ++++++- .../kontres/test_bookable_item_integration.py | 80 +++++++++++++++++++ .../kontres/test_reservation_integration.py | 78 ++++++++++++++++++ app/tests/kontres/test_reservation_model.py | 23 ------ 7 files changed, 273 insertions(+), 26 deletions(-) create mode 100644 app/kontres/migrations/0005_bookableitem_allows_alcohol_and_more.py create mode 100644 app/tests/kontres/test_bookable_item_integration.py diff --git a/app/kontres/migrations/0005_bookableitem_allows_alcohol_and_more.py b/app/kontres/migrations/0005_bookableitem_allows_alcohol_and_more.py new file mode 100644 index 00000000..90430825 --- /dev/null +++ b/app/kontres/migrations/0005_bookableitem_allows_alcohol_and_more.py @@ -0,0 +1,57 @@ +# Generated by Django 4.2.5 on 2024-03-05 18:11 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ("kontres", "0004_reservation_group"), + ] + + operations = [ + migrations.AddField( + model_name="bookableitem", + name="allows_alcohol", + field=models.BooleanField(default=False), + ), + migrations.AddField( + model_name="reservation", + name="alcohol_agreement", + field=models.BooleanField(default=False), + ), + migrations.AddField( + model_name="reservation", + name="sober_watch", + field=models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name="sober_watch_reservations", + to=settings.AUTH_USER_MODEL, + ), + ), + migrations.AlterField( + model_name="reservation", + name="author", + field=models.ForeignKey( + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name="reservations", + to=settings.AUTH_USER_MODEL, + ), + ), + migrations.AlterField( + model_name="reservation", + name="bookable_item", + field=models.ForeignKey( + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name="reservations", + to="kontres.bookableitem", + ), + ), + ] diff --git a/app/kontres/models/bookable_item.py b/app/kontres/models/bookable_item.py index fc2ece6f..6ad0ece1 100644 --- a/app/kontres/models/bookable_item.py +++ b/app/kontres/models/bookable_item.py @@ -13,6 +13,7 @@ class BookableItem(BaseModel, BasePermissionModel): id = models.UUIDField(primary_key=True, default=uuid.uuid4, editable=False) name = models.CharField(max_length=20) description = models.TextField(blank=True) + allows_alcohol = models.BooleanField(default=False) @classmethod def has_read_permission(cls, request): @@ -34,5 +35,14 @@ def has_create_permission(cls, request): def has_update_permission(cls, request): return check_has_access(cls.write_access, request) + def has_object_destroy_permission(self, request): + return self.check_has_admin_permission(request) + + def has_object_update_permission(self, request): + return self.check_has_admin_permission(request) + + def check_has_admin_permission(self, request): + return check_has_access([AdminGroup.INDEX, AdminGroup.HS], request) + def __str__(self): return self.name diff --git a/app/kontres/models/reservation.py b/app/kontres/models/reservation.py index 1a7260ba..1ceb5af5 100644 --- a/app/kontres/models/reservation.py +++ b/app/kontres/models/reservation.py @@ -16,10 +16,18 @@ class Reservation(BaseModel, BasePermissionModel): write_access = [Groups.TIHLDE] id = models.UUIDField(primary_key=True, default=uuid.uuid4, editable=False) author = models.ForeignKey( - User, on_delete=models.CASCADE, related_name="reservations" + User, + on_delete=models.SET_NULL, + related_name="reservations", + null=True, + blank=False, ) bookable_item = models.ForeignKey( - BookableItem, on_delete=models.PROTECT, related_name="reservations" + BookableItem, + on_delete=models.SET_NULL, + related_name="reservations", + null=True, + blank=False, ) start_time = models.DateTimeField() end_time = models.DateTimeField() @@ -37,6 +45,14 @@ class Reservation(BaseModel, BasePermissionModel): null=True, blank=True, ) + alcohol_agreement = models.BooleanField(default=False) + sober_watch = models.ForeignKey( + User, + on_delete=models.SET_NULL, + related_name="sober_watch_reservations", + null=True, + blank=True, + ) def __str__(self): return f"{self.state} - Reservation request by {self.author.first_name} {self.author.last_name} to book {self.bookable_item.name}. Created at {self.created_at}" diff --git a/app/kontres/serializer/reservation_seralizer.py b/app/kontres/serializer/reservation_seralizer.py index 5be0b977..c178c1dc 100644 --- a/app/kontres/serializer/reservation_seralizer.py +++ b/app/kontres/serializer/reservation_seralizer.py @@ -38,18 +38,47 @@ class Meta: def validate(self, data): user = self.context["request"].user group = data.get("group", None) + + bookable_item = ( + data.get("bookable_item") + if "bookable_item" in data + else self.instance.bookable_item + ) + if group: self.validate_group(group) + + if bookable_item.allows_alcohol: + self.validate_alcohol(data) + self.validate_state_change(data, user) self.validate_time_and_overlapping(data) return data + def validate_alcohol(self, data): + if not data.get( + "alcohol_agreement", + self.instance.alcohol_agreement if self.instance else False, + ): + raise serializers.ValidationError( + "Du må godta at dere vil følge reglene for alkoholbruk." + ) + sober_watch = data.get( + "sober_watch", self.instance.sober_watch if self.instance else None + ) + if ( + not sober_watch + or not User.objects.filter(user_id=sober_watch.user_id).exists() + ): + raise serializers.ValidationError( + "Du må velge en edruvakt for reservasjonen." + ) + def validate_group(self, value): user = self.context["request"].user group = value if self.instance and group != self.instance.group: - # Assuming your model logic and permissions are correctly implemented in is_HS_or_Index_member if ( not user.is_HS_or_Index_member and self.instance.state != ReservationStateEnum.PENDING diff --git a/app/tests/kontres/test_bookable_item_integration.py b/app/tests/kontres/test_bookable_item_integration.py new file mode 100644 index 00000000..ec1cbab9 --- /dev/null +++ b/app/tests/kontres/test_bookable_item_integration.py @@ -0,0 +1,80 @@ +from rest_framework import status + +import pytest + +from app.util.test_utils import get_api_client + + +@pytest.mark.django_db +def test_unauthenticated_request_cannot_create_bookable_item(): + client = get_api_client() + response = client.post("/kontres/bookable_items/", {"name": "test"}, format="json") + assert response.status_code == status.HTTP_403_FORBIDDEN + + +@pytest.mark.django_db +def test_admin_can_delete_bookable_item(admin_user, bookable_item): + client = get_api_client(user=admin_user) + response = client.delete( + f"/kontres/bookable_items/{bookable_item.id}/", format="json" + ) + assert response.status_code == status.HTTP_204_NO_CONTENT + + +@pytest.mark.django_db +def test_member_cannot_delete_bookable_item(member, bookable_item): + client = get_api_client(user=member) + response = client.delete( + f"/kontres/bookable_items/{bookable_item.id}/", format="json" + ) + assert response.status_code == status.HTTP_403_FORBIDDEN + + +@pytest.mark.django_db +def test_delete_bookable_item_sets_reservation_bookable_item_to_null( + admin_user, bookable_item, reservation +): + # Ensure the bookable_item is part of the reservation + reservation.bookable_item = bookable_item + reservation.save() + + client = get_api_client(user=admin_user) + response = client.delete( + f"/kontres/bookable_items/{bookable_item.id}/", format="json" + ) + + # Refresh the reservation from the database to check the updated state + reservation.refresh_from_db() + + # The deletion should succeed + assert response.status_code == 204, "Expected successful deletion of bookable item." + + # After deletion, the reservation's bookable_item should be set to null + assert ( + reservation.bookable_item is None + ), "Expected reservation.bookable_item to be set to null after bookable item deletion." + + +@pytest.mark.django_db +def test_delete_bookable_item_with_invalid_id(admin_user): + client = get_api_client(user=admin_user) + invalid_id = 99999 + response = client.delete(f"/kontres/bookable_items/{invalid_id}/", format="json") + assert response.status_code == status.HTTP_404_NOT_FOUND + + +@pytest.mark.django_db +def test_member_cannot_edit_bookable_item(member, bookable_item): + client = get_api_client(user=member) + response = client.put("/kontres/bookable_items/", {"name": "test"}, format="json") + assert response.status_code == status.HTTP_403_FORBIDDEN + + +@pytest.mark.django_db +def test_admin_can_edit_bookable_item(admin_user, bookable_item): + client = get_api_client(user=admin_user) + response = client.put( + f"/kontres/bookable_items/{bookable_item.id}/", {"name": "test"}, format="json" + ) + assert response.status_code == status.HTTP_200_OK + assert response.data["name"] == "test" diff --git a/app/tests/kontres/test_reservation_integration.py b/app/tests/kontres/test_reservation_integration.py index 69c0a559..a97ee8fc 100644 --- a/app/tests/kontres/test_reservation_integration.py +++ b/app/tests/kontres/test_reservation_integration.py @@ -36,6 +36,84 @@ def test_member_can_create_reservation(member, bookable_item): assert response.data["state"] == "PENDING" +@pytest.mark.django_db +def test_member_can_create_reservation_with_alcohol_agreement(member, bookable_item): + client = get_api_client(user=member) + + bookable_item.allows_alcohol = True + bookable_item.save() + + response = client.post( + "/kontres/reservations/", + { + "bookable_item": bookable_item.id, + "start_time": "2030-10-10T10:00:00Z", + "end_time": "2030-10-10T11:00:00Z", + "alcohol_agreement": True, + "sober_watch": member.user_id, + }, + format="json", + ) + + assert response.status_code == 201, response.data + assert response.data.get("alcohol_agreement") is True + assert response.data.get("sober_watch") == str(member.user_id) + + +@pytest.mark.django_db +def test_reservation_creation_fails_without_alcohol_agreement(member, bookable_item): + client = get_api_client(user=member) + + bookable_item.allows_alcohol = True + bookable_item.save() + + response = client.post( + "/kontres/reservations/", + { + "bookable_item": bookable_item.id, + "start_time": "2030-10-10T10:00:00Z", + "end_time": "2030-10-10T11:00:00Z", + # Notice the absence of "alcohol_agreement": True, + "sober_watch": member.user_id, + }, + format="json", + ) + + assert response.status_code == 400 + expected_error_message = "Du må godta at dere vil følge reglene for alkoholbruk." + actual_error_messages = response.data.get("non_field_errors", []) + assert any( + expected_error_message in error for error in actual_error_messages + ), f"Expected specific alcohol agreement validation error: {expected_error_message}" + + +@pytest.mark.django_db +def test_reservation_creation_fails_without_sober_watch(member, bookable_item): + client = get_api_client(user=member) + + bookable_item.allows_alcohol = True + bookable_item.save() + + response = client.post( + "/kontres/reservations/", + { + "bookable_item": bookable_item.id, + "start_time": "2030-10-10T10:00:00Z", + "end_time": "2030-10-10T11:00:00Z", + "alcohol_agreement": True, + # Notice the absence of "sober_watch", + }, + format="json", + ) + + assert response.status_code == 400 + expected_error_message = "Du må velge en edruvakt for reservasjonen." + actual_error_messages = response.data.get("non_field_errors", []) + assert any( + expected_error_message in error for error in actual_error_messages + ), f"Expected specific alcohol agreement validation error: {expected_error_message}" + + @pytest.mark.django_db def test_member_cannot_set_different_author_in_reservation( member, bookable_item, sosialen_user diff --git a/app/tests/kontres/test_reservation_model.py b/app/tests/kontres/test_reservation_model.py index cf9d8ecb..72728882 100644 --- a/app/tests/kontres/test_reservation_model.py +++ b/app/tests/kontres/test_reservation_model.py @@ -1,4 +1,3 @@ -from django.db.utils import IntegrityError from django.utils import timezone import pytest @@ -59,17 +58,6 @@ def test_state_transitions(reservation): assert reservation.state == ReservationStateEnum.CANCELLED -@pytest.mark.django_db -def test_reservation_requires_bookable_item(): - with pytest.raises(IntegrityError): - user = User.objects.create(user_id="test_user", email="test@test.com") - Reservation.objects.create( - start_time=timezone.now(), - end_time=timezone.now() + timezone.timedelta(hours=1), - author=user, - ) - - @pytest.mark.django_db def test_created_at_field(): user = User.objects.create(user_id="test_user", email="test@test.com") @@ -104,17 +92,6 @@ def test_multiple_reservations(): assert reservation2 is not None -@pytest.mark.django_db -def test_reservation_requires_author(): - with pytest.raises(IntegrityError): - bookable_item = BookableItem.objects.create(name="Test Item") - Reservation.objects.create( - start_time=timezone.now(), - end_time=timezone.now() + timezone.timedelta(hours=1), - bookable_item=bookable_item, - ) - - @pytest.mark.django_db def test_reservation_with_group(group): user = User.objects.create(user_id="test_user") From 2921bd949d4d7562c3f545af4e17a3e9ecf3338e Mon Sep 17 00:00:00 2001 From: Mads Nylund <73914541+MadsNyl@users.noreply.github.com> Date: Fri, 8 Mar 2024 18:28:57 +0100 Subject: [PATCH 17/23] Removed SQL logging settings (#776) removed logging settings --- app/settings.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/app/settings.py b/app/settings.py index e44640aa..9427eee6 100644 --- a/app/settings.py +++ b/app/settings.py @@ -272,12 +272,13 @@ "formatter": "verbose", }, }, - "loggers": { - "django": { - "propagate": True, - "level": "DEBUG", - }, - }, + # REMOVE COMMENTS TO ADD SQL LOGGING + # "loggers": { + # "django": { + # "propagate": True, + # "level": "DEBUG", + # }, + # }, "root": { "handlers": ["file"], }, From 1f70949243d14562d4942b6376cc17bf812428ff Mon Sep 17 00:00:00 2001 From: Mads Nylund <73914541+MadsNyl@users.noreply.github.com> Date: Mon, 11 Mar 2024 14:20:27 +0100 Subject: [PATCH 18/23] fixed bug of payment countdown for registrations from waitlist to queue (#778) --- app/content/models/registration.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/app/content/models/registration.py b/app/content/models/registration.py index 4cb84d5e..795bdc9c 100644 --- a/app/content/models/registration.py +++ b/app/content/models/registration.py @@ -132,10 +132,7 @@ def delete(self, *args, **kwargs): if moved_registration: moved_registration.save() - if ( - moved_registration.event.is_paid_event - and not moved_registration.is_on_wait - ): + if moved_registration.event.is_paid_event: try: start_payment_countdown( moved_registration.event, moved_registration From 52a1474208caa9a06169df9e143478b98f1ad2d5 Mon Sep 17 00:00:00 2001 From: Mads Nylund <73914541+MadsNyl@users.noreply.github.com> Date: Mon, 11 Mar 2024 15:37:21 +0100 Subject: [PATCH 19/23] Update CHANGELOG.md (#779) --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a11b22d6..b526f705 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,12 @@ ## Neste versjon +## Versjon 2023.03.11 +- 🦟 **Vipps** Brukere som kommer fra venteliste vil nå få en payment countdown startet, slik at de blir kastet ut hvis de ikke betaler. +- ⚡ **Venteliste** Brukere vil nå se sin reelle ventelisteplass som tar hensyn til prioriteringer. +- 🎨 **Logging** SQL Debug for pytest er skrudd av. +- ✨ **Kontres** Endepunkter for reservasjoner av utstyr og kontor. + ## Versjon 2023.02.07 - 🦟 **Vipps** Brukere kan nå oppdatere betalt arrangement, uten at det betalte arrangementet blir slettet. From 93dfcd6771a779bf4bf0af572e89d5498c0df320 Mon Sep 17 00:00:00 2001 From: Erik Skjellevik <98759397+eriskjel@users.noreply.github.com> Date: Mon, 11 Mar 2024 17:28:49 +0100 Subject: [PATCH 20/23] Feat(kontres)/add endpoint for my reservations (#777) * added endpoint to fetch /me/reservations * added tests for /me/reservations endpoint * removed logging statement * admin can now fetch all reservations by user * created tests for admin fetching reservations by user --- app/content/views/user.py | 11 +++ app/kontres/views/reservation.py | 15 +++- .../kontres/test_reservation_integration.py | 88 +++++++++++++++++++ 3 files changed, 111 insertions(+), 3 deletions(-) diff --git a/app/content/views/user.py b/app/content/views/user.py index c1d8ba67..e0edc677 100644 --- a/app/content/views/user.py +++ b/app/content/views/user.py @@ -37,6 +37,8 @@ MembershipHistorySerializer, MembershipSerializer, ) +from app.kontres.models.reservation import Reservation +from app.kontres.serializer.reservation_seralizer import ReservationSerializer from app.util.export_user_data import export_user_data from app.util.utils import CaseInsensitiveBooleanQueryParam @@ -378,3 +380,12 @@ def export_user_data(self, request, *args, **kwargs): {"detail": "Noe gikk galt, prøv igjen senere eller kontakt Index"}, status=status.HTTP_500_INTERNAL_SERVER_ERROR, ) + + @action(detail=False, methods=["get"], url_path="me/reservations") + def get_user_reservations(self, request, *args, **kwargs): + user = request.user + reservations = Reservation.objects.filter(author=user).order_by("start_time") + serializer = ReservationSerializer( + reservations, many=True, context={"request": request} + ) + return Response(serializer.data, status=status.HTTP_200_OK) diff --git a/app/kontres/views/reservation.py b/app/kontres/views/reservation.py index eeeb8721..49e67e44 100644 --- a/app/kontres/views/reservation.py +++ b/app/kontres/views/reservation.py @@ -1,6 +1,7 @@ from django.db.models import Q from django.utils.dateparse import parse_datetime from rest_framework import status +from rest_framework.exceptions import PermissionDenied from rest_framework.response import Response from app.common.permissions import BasicViewPermission @@ -17,21 +18,29 @@ class ReservationViewSet(BaseViewSet): def get_queryset(self): start_date = self.request.GET.get("start_date") end_date = self.request.GET.get("end_date") + user_id = self.request.query_params.get("user_id") + queryset = Reservation.objects.all() - # Convert string dates to datetime objects if start_date: start_date = parse_datetime(start_date) if end_date: end_date = parse_datetime(end_date) - # Adjusted filter to capture overlapping reservations if start_date and end_date: queryset = Reservation.objects.filter( Q(start_time__lt=end_date) & Q(end_time__gt=start_date) ) return queryset - return Reservation.objects.all() + if user_id: + if self.request.user.is_HS_or_Index_member: + queryset = queryset.filter(author__user_id=user_id) + else: + raise PermissionDenied( + "Du har ikke tilgang til å se andres reservasjoner." + ) + + return queryset def create(self, request, *args, **kwargs): serializer = ReservationSerializer( diff --git a/app/tests/kontres/test_reservation_integration.py b/app/tests/kontres/test_reservation_integration.py index a97ee8fc..5666ab2f 100644 --- a/app/tests/kontres/test_reservation_integration.py +++ b/app/tests/kontres/test_reservation_integration.py @@ -840,3 +840,91 @@ def test_user_cannot_change_reservation_group_if_state_is_not_pending( ) assert response.status_code == status.HTTP_400_BAD_REQUEST + + +@pytest.mark.django_db +def test_user_can_fetch_own_reservations(member, reservation): + client = get_api_client(user=member) + + reservation.author = member + reservation.save() + + response = client.get("/users/me/reservations/") + + assert response.status_code == 200 + assert all( + reservation["author_detail"]["user_id"] == str(member.user_id) + for reservation in response.data + ) + + +@pytest.mark.django_db +def test_user_reservations_endpoint_returns_correct_reservations( + member, bookable_item, reservation +): + client = get_api_client(user=member) + + Reservation.objects.bulk_create( + [ + Reservation( + author=member, + bookable_item=bookable_item, + start_time=f"2030-10-10T1{num}:00:00Z", + end_time=f"2030-10-10T1{num + 1}:00:00Z", + description=f"Test reservation {num}", + ) + for num in range(3) + ] + ) + + response = client.get("/users/me/reservations/") + + assert response.status_code == 200 + assert len(response.data) == 3 + + fixture_reservation_id = str(reservation.id) + returned_reservation_ids = [res["id"] for res in response.data] + assert fixture_reservation_id not in returned_reservation_ids + + +@pytest.mark.django_db +def test_admin_can_fetch_reservations_for_specific_user( + admin_user, member, bookable_item +): + client = get_api_client(user=admin_user) + + Reservation.objects.bulk_create( + [ + Reservation( + author=member, + bookable_item=bookable_item, + start_time=f"2030-10-{10 + num}T10:00:00Z", + end_time=f"2030-10-{10 + num}T11:00:00Z", + description=f"Member's reservation {num}", + ) + for num in range(3) # Create 3 reservations for the member + ] + ) + + created_reservations = Reservation.objects.filter(author=member).order_by( + "start_time" + ) + created_reservation_ids = {str(res.id) for res in created_reservations} + + response = client.get(f"/kontres/reservations/?user_id={member.user_id}") + + assert response.status_code == 200 + assert len(response.data) == 3 + + response_reservation_ids = {res["id"] for res in response.data} + + assert created_reservation_ids == response_reservation_ids + + +@pytest.mark.django_db +def test_member_cannot_fetch_reservations_for_specific_user(member): + client = get_api_client(user=member) + + response = client.get(f"/kontres/reservations/?user_id={member.user_id}") + + assert response.status_code == 403 From 2fd959694ec52fe38add8ffd74795c5fcb7bbcdd Mon Sep 17 00:00:00 2001 From: Erik Skjellevik <98759397+eriskjel@users.noreply.github.com> Date: Mon, 18 Mar 2024 17:43:45 +0100 Subject: [PATCH 21/23] Fix(kontres)/fix overlapping and alcohol issues (#782) * serializer will now only raise overlapping error if the reservation is confirmed * created tests for new overlap fix * created destroy endpoint * modified destroy endpoint status codes * removed destroy * linting * renamed alcohol_agreement to serves_alcohol * new serializer logic for alcohol validaiton * updated tests with new alcohol logic --- ...ol_agreement_reservation_serves_alcohol.py | 18 +++ app/kontres/models/reservation.py | 2 +- .../serializer/reservation_seralizer.py | 30 ++-- app/kontres/views/reservation.py | 2 +- .../kontres/test_bookable_item_integration.py | 37 +++++ .../kontres/test_reservation_integration.py | 145 +++++++++++++----- 6 files changed, 180 insertions(+), 54 deletions(-) create mode 100644 app/kontres/migrations/0006_rename_alcohol_agreement_reservation_serves_alcohol.py diff --git a/app/kontres/migrations/0006_rename_alcohol_agreement_reservation_serves_alcohol.py b/app/kontres/migrations/0006_rename_alcohol_agreement_reservation_serves_alcohol.py new file mode 100644 index 00000000..fdc3de0a --- /dev/null +++ b/app/kontres/migrations/0006_rename_alcohol_agreement_reservation_serves_alcohol.py @@ -0,0 +1,18 @@ +# Generated by Django 4.2.5 on 2024-03-18 15:24 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ("kontres", "0005_bookableitem_allows_alcohol_and_more"), + ] + + operations = [ + migrations.RenameField( + model_name="reservation", + old_name="alcohol_agreement", + new_name="serves_alcohol", + ), + ] diff --git a/app/kontres/models/reservation.py b/app/kontres/models/reservation.py index 1ceb5af5..50d15921 100644 --- a/app/kontres/models/reservation.py +++ b/app/kontres/models/reservation.py @@ -45,7 +45,7 @@ class Reservation(BaseModel, BasePermissionModel): null=True, blank=True, ) - alcohol_agreement = models.BooleanField(default=False) + serves_alcohol = models.BooleanField(default=False) sober_watch = models.ForeignKey( User, on_delete=models.SET_NULL, diff --git a/app/kontres/serializer/reservation_seralizer.py b/app/kontres/serializer/reservation_seralizer.py index c178c1dc..86f7200d 100644 --- a/app/kontres/serializer/reservation_seralizer.py +++ b/app/kontres/serializer/reservation_seralizer.py @@ -56,23 +56,20 @@ def validate(self, data): return data def validate_alcohol(self, data): - if not data.get( - "alcohol_agreement", - self.instance.alcohol_agreement if self.instance else False, + if data.get( + "serves_alcohol", + self.instance.serves_alcohol if self.instance else False, ): - raise serializers.ValidationError( - "Du må godta at dere vil følge reglene for alkoholbruk." - ) - sober_watch = data.get( - "sober_watch", self.instance.sober_watch if self.instance else None - ) - if ( - not sober_watch - or not User.objects.filter(user_id=sober_watch.user_id).exists() - ): - raise serializers.ValidationError( - "Du må velge en edruvakt for reservasjonen." + sober_watch = data.get( + "sober_watch", self.instance.sober_watch if self.instance else None ) + if ( + not sober_watch + or not User.objects.filter(user_id=sober_watch.user_id).exists() + ): + raise serializers.ValidationError( + "Du må velge en edruvakt for reservasjonen." + ) def validate_group(self, value): user = self.context["request"].user @@ -104,7 +101,6 @@ def validate_state_change(self, data, user): "state": "Du har ikke rettigheter til å endre reservasjonsstatusen." } ) - pass def validate_time_and_overlapping(self, data): @@ -140,6 +136,7 @@ def validate_time_and_overlapping(self, data): bookable_item=bookable_item, end_time__gt=start_time, start_time__lt=end_time, + state=ReservationStateEnum.CONFIRMED, ) # Exclude the current instance if updating if self.instance: @@ -149,4 +146,3 @@ def validate_time_and_overlapping(self, data): raise serializers.ValidationError( "Det er en reservasjonsoverlapp for det gitte tidsrommet." ) - pass diff --git a/app/kontres/views/reservation.py b/app/kontres/views/reservation.py index 49e67e44..af7defee 100644 --- a/app/kontres/views/reservation.py +++ b/app/kontres/views/reservation.py @@ -63,4 +63,4 @@ def update(self, request, *args, **kwargs): def destroy(self, request, *args, **kwargs): super().destroy(self, request, *args, **kwargs) - return Response(status=status.HTTP_204_NO_CONTENT) + return Response(status=status.HTTP_200_OK) diff --git a/app/tests/kontres/test_bookable_item_integration.py b/app/tests/kontres/test_bookable_item_integration.py index ec1cbab9..3fe597bc 100644 --- a/app/tests/kontres/test_bookable_item_integration.py +++ b/app/tests/kontres/test_bookable_item_integration.py @@ -2,6 +2,7 @@ import pytest +from app.kontres.models.bookable_item import BookableItem from app.util.test_utils import get_api_client @@ -78,3 +79,39 @@ def test_admin_can_edit_bookable_item(admin_user, bookable_item): ) assert response.status_code == status.HTTP_200_OK assert response.data["name"] == "test" + + +@pytest.mark.django_db +def test_get_returns_empty_list_when_no_bookable_items(member): + client = get_api_client(user=member) + response = client.get("/kontres/bookable_items/", format="json") + + assert response.status_code == status.HTTP_200_OK + assert ( + response.data == [] + ), "Expected an empty list when there are no bookable items" + + +@pytest.mark.django_db +def admin_can_delete_bookable_item(member, bookable_item): + client = get_api_client(user=member) + + # Verify the item exists before deletion + exists_before_delete = BookableItem.objects.filter(id=bookable_item.id).exists() + assert ( + exists_before_delete is True + ), "The bookable item should exist in the database before deletion." + + # Perform the deletion request + response = client.delete( + f"/kontres/bookable_items/{bookable_item.id}/", format="json" + ) + + # Check the response status code to ensure the request was processed as expected + assert response.status_code == status.HTTP_200_OK + + # Verify the item does not exist after deletion + exists_after_delete = BookableItem.objects.filter(id=bookable_item.id).exists() + assert ( + exists_after_delete is True + ), "The bookable item should not exist in the database after deletion." diff --git a/app/tests/kontres/test_reservation_integration.py b/app/tests/kontres/test_reservation_integration.py index 5666ab2f..1c5208ac 100644 --- a/app/tests/kontres/test_reservation_integration.py +++ b/app/tests/kontres/test_reservation_integration.py @@ -37,7 +37,7 @@ def test_member_can_create_reservation(member, bookable_item): @pytest.mark.django_db -def test_member_can_create_reservation_with_alcohol_agreement(member, bookable_item): +def test_member_can_create_reservation_with_alcohol(member, bookable_item): client = get_api_client(user=member) bookable_item.allows_alcohol = True @@ -49,44 +49,17 @@ def test_member_can_create_reservation_with_alcohol_agreement(member, bookable_i "bookable_item": bookable_item.id, "start_time": "2030-10-10T10:00:00Z", "end_time": "2030-10-10T11:00:00Z", - "alcohol_agreement": True, + "serves_alcohol": True, "sober_watch": member.user_id, }, format="json", ) assert response.status_code == 201, response.data - assert response.data.get("alcohol_agreement") is True + assert response.data.get("serves_alcohol") is True assert response.data.get("sober_watch") == str(member.user_id) -@pytest.mark.django_db -def test_reservation_creation_fails_without_alcohol_agreement(member, bookable_item): - client = get_api_client(user=member) - - bookable_item.allows_alcohol = True - bookable_item.save() - - response = client.post( - "/kontres/reservations/", - { - "bookable_item": bookable_item.id, - "start_time": "2030-10-10T10:00:00Z", - "end_time": "2030-10-10T11:00:00Z", - # Notice the absence of "alcohol_agreement": True, - "sober_watch": member.user_id, - }, - format="json", - ) - - assert response.status_code == 400 - expected_error_message = "Du må godta at dere vil følge reglene for alkoholbruk." - actual_error_messages = response.data.get("non_field_errors", []) - assert any( - expected_error_message in error for error in actual_error_messages - ), f"Expected specific alcohol agreement validation error: {expected_error_message}" - - @pytest.mark.django_db def test_reservation_creation_fails_without_sober_watch(member, bookable_item): client = get_api_client(user=member) @@ -100,14 +73,15 @@ def test_reservation_creation_fails_without_sober_watch(member, bookable_item): "bookable_item": bookable_item.id, "start_time": "2030-10-10T10:00:00Z", "end_time": "2030-10-10T11:00:00Z", - "alcohol_agreement": True, + "serves_alcohol": True, # Notice the absence of "sober_watch", }, format="json", ) assert response.status_code == 400 - expected_error_message = "Du må velge en edruvakt for reservasjonen." + print(response.data) + expected_error_message = "Du må velge en edruvakt for reservasjonen." actual_error_messages = response.data.get("non_field_errors", []) assert any( expected_error_message in error for error in actual_error_messages @@ -185,7 +159,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 @pytest.mark.django_db @@ -399,7 +373,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 @@ -501,7 +475,9 @@ def test_unauthenticated_request_cannot_create_reservation(bookable_item): @pytest.mark.django_db -def test_creating_overlapping_reservation(member, bookable_item, admin_user): +def test_creating_overlapping_reservation_should_not_work_when_confirmed( + member, bookable_item, admin_user +): # Create a confirmed reservation using the ReservationFactory existing_confirmed_reservation = ReservationFactory( bookable_item=bookable_item, @@ -532,6 +508,105 @@ def test_creating_overlapping_reservation(member, bookable_item, admin_user): assert response.status_code == status.HTTP_400_BAD_REQUEST +@pytest.mark.django_db +def test_updating_to_overlapping_reservation_should_not_work_when_confirmed( + member, bookable_item, admin_user +): + # Create two initial reservations, one confirmed and one pending + confirmed_reservation = ReservationFactory( + bookable_item=bookable_item, + start_time=timezone.now() + timezone.timedelta(hours=1), + end_time=timezone.now() + timezone.timedelta(hours=2), + state=ReservationStateEnum.CONFIRMED, + ) + + pending_reservation = ReservationFactory( + bookable_item=bookable_item, + start_time=confirmed_reservation.start_time + + timezone.timedelta(minutes=30), # Overlapping time + end_time=confirmed_reservation.end_time + timezone.timedelta(minutes=30), + state=ReservationStateEnum.PENDING, + ) + + # Now attempt to update the pending reservation to confirmed, which should overlap with the confirmed_reservation + client = get_api_client(user=member) + response = client.put( + f"/kontres/reservations/{pending_reservation.id}/", + {"state": ReservationStateEnum.CONFIRMED}, + format="json", + ) + + # The system should not allow this, as it would overlap with another confirmed reservation + assert ( + response.status_code == status.HTTP_400_BAD_REQUEST + ), "Should not update reservation to confirmed due to overlap" + + +@pytest.mark.django_db +def test_creating_overlapping_reservation_should_work_when_cancelled( + member, bookable_item, admin_user +): + existing_confirmed_reservation = ReservationFactory( + bookable_item=bookable_item, + start_time=timezone.now() + timezone.timedelta(hours=1), + end_time=timezone.now() + timezone.timedelta(hours=2), + state=ReservationStateEnum.CANCELLED, # Set the reservation as declined + ) + + # Now attempt to create an overlapping reservation + client = get_api_client(user=member) + overlapping_start_time = ( + existing_confirmed_reservation.start_time + timezone.timedelta(minutes=30) + ) + response = client.post( + "/kontres/reservations/", + { + "author": member.user_id, + "bookable_item": bookable_item.id, + "start_time": overlapping_start_time, + "end_time": existing_confirmed_reservation.end_time + + timezone.timedelta(hours=1), + "state": ReservationStateEnum.PENDING, + }, + format="json", + ) + + assert response.status_code == status.HTTP_201_CREATED + + +@pytest.mark.django_db +def test_creating_overlapping_reservation_should_work_when_pending( + member, bookable_item, admin_user +): + # Create a confirmed reservation using the ReservationFactory + existing_confirmed_reservation = ReservationFactory( + bookable_item=bookable_item, + start_time=timezone.now() + timezone.timedelta(hours=1), + end_time=timezone.now() + timezone.timedelta(hours=2), + state=ReservationStateEnum.PENDING, # Set the reservation as declined + ) + + # Now attempt to create an overlapping reservation + client = get_api_client(user=member) + overlapping_start_time = ( + existing_confirmed_reservation.start_time + timezone.timedelta(minutes=30) + ) + response = client.post( + "/kontres/reservations/", + { + "author": member.user_id, + "bookable_item": bookable_item.id, + "start_time": overlapping_start_time, + "end_time": existing_confirmed_reservation.end_time + + timezone.timedelta(hours=1), + "state": ReservationStateEnum.PENDING, + }, + format="json", + ) + + assert response.status_code == status.HTTP_201_CREATED + + @pytest.mark.django_db def test_retrieve_specific_reservation_within_its_date_range(member, bookable_item): client = get_api_client(user=member) From ff6eade48289e7f2127055d5f15740b9829328a8 Mon Sep 17 00:00:00 2001 From: Erik Skjellevik <98759397+eriskjel@users.noreply.github.com> Date: Mon, 18 Mar 2024 21:00:08 +0100 Subject: [PATCH 22/23] Fix(kontres)/return full sober watch (#783) * updated serializer to include full sober watch user object * updated test to account for new object --- app/kontres/serializer/reservation_seralizer.py | 5 +++++ app/tests/kontres/test_reservation_integration.py | 6 ++++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/app/kontres/serializer/reservation_seralizer.py b/app/kontres/serializer/reservation_seralizer.py index 86f7200d..af52fc37 100644 --- a/app/kontres/serializer/reservation_seralizer.py +++ b/app/kontres/serializer/reservation_seralizer.py @@ -31,6 +31,11 @@ class ReservationSerializer(serializers.ModelSerializer): ) author_detail = UserSerializer(source="author", read_only=True) + sober_watch = serializers.PrimaryKeyRelatedField( + queryset=User.objects.all(), write_only=True, required=False + ) + sober_watch_detail = UserSerializer(source="sober_watch", read_only=True) + class Meta: model = Reservation fields = "__all__" diff --git a/app/tests/kontres/test_reservation_integration.py b/app/tests/kontres/test_reservation_integration.py index 1c5208ac..809effef 100644 --- a/app/tests/kontres/test_reservation_integration.py +++ b/app/tests/kontres/test_reservation_integration.py @@ -37,7 +37,9 @@ def test_member_can_create_reservation(member, bookable_item): @pytest.mark.django_db -def test_member_can_create_reservation_with_alcohol(member, bookable_item): +def test_member_can_create_reservation_with_alcohol_and_sober_watch( + member, bookable_item +): client = get_api_client(user=member) bookable_item.allows_alcohol = True @@ -57,7 +59,7 @@ def test_member_can_create_reservation_with_alcohol(member, bookable_item): assert response.status_code == 201, response.data assert response.data.get("serves_alcohol") is True - assert response.data.get("sober_watch") == str(member.user_id) + assert response.data["sober_watch_detail"]["user_id"] == str(member.user_id) @pytest.mark.django_db From 856de6c1ad73dcdf0e4a68b9043c3766215162d9 Mon Sep 17 00:00:00 2001 From: Erik Skjellevik <98759397+eriskjel@users.noreply.github.com> Date: Tue, 19 Mar 2024 14:01:13 +0100 Subject: [PATCH 23/23] Fix(kontres)/custom delete message (#784) * added custom delete messages to reservation and bookable item viewset * modified test to account for custom delete message, and deleted duplicated test --- app/kontres/views/bookable_item.py | 9 +++++++ app/kontres/views/reservation.py | 4 ++- .../kontres/test_bookable_item_integration.py | 27 +------------------ .../kontres/test_reservation_integration.py | 5 ++-- 4 files changed, 16 insertions(+), 29 deletions(-) diff --git a/app/kontres/views/bookable_item.py b/app/kontres/views/bookable_item.py index 246937ca..cb27ed72 100644 --- a/app/kontres/views/bookable_item.py +++ b/app/kontres/views/bookable_item.py @@ -1,3 +1,6 @@ +from rest_framework import status +from rest_framework.response import Response + from app.common.permissions import BasicViewPermission from app.common.viewsets import BaseViewSet from app.kontres.models.bookable_item import BookableItem @@ -10,3 +13,9 @@ class BookableItemViewSet(BaseViewSet): queryset = BookableItem.objects.all() serializer_class = BookableItemSerializer permission_classes = [BasicViewPermission] + + def destroy(self, request, *args, **kwargs): + super().destroy(self, request, *args, **kwargs) + return Response( + {"detail": "Gjenstanden ble slettet."}, status=status.HTTP_204_NO_CONTENT + ) diff --git a/app/kontres/views/reservation.py b/app/kontres/views/reservation.py index af7defee..cfd75f96 100644 --- a/app/kontres/views/reservation.py +++ b/app/kontres/views/reservation.py @@ -63,4 +63,6 @@ def update(self, request, *args, **kwargs): def destroy(self, request, *args, **kwargs): super().destroy(self, request, *args, **kwargs) - return Response(status=status.HTTP_200_OK) + return Response( + {"detail": "Reservasjonen ble slettet."}, status=status.HTTP_204_NO_CONTENT + ) diff --git a/app/tests/kontres/test_bookable_item_integration.py b/app/tests/kontres/test_bookable_item_integration.py index 3fe597bc..e51a4bb5 100644 --- a/app/tests/kontres/test_bookable_item_integration.py +++ b/app/tests/kontres/test_bookable_item_integration.py @@ -2,7 +2,6 @@ import pytest -from app.kontres.models.bookable_item import BookableItem from app.util.test_utils import get_api_client @@ -20,6 +19,7 @@ def test_admin_can_delete_bookable_item(admin_user, bookable_item): f"/kontres/bookable_items/{bookable_item.id}/", format="json" ) assert response.status_code == status.HTTP_204_NO_CONTENT + assert response.data["detail"] == "Gjenstanden ble slettet." @pytest.mark.django_db @@ -90,28 +90,3 @@ def test_get_returns_empty_list_when_no_bookable_items(member): assert ( response.data == [] ), "Expected an empty list when there are no bookable items" - - -@pytest.mark.django_db -def admin_can_delete_bookable_item(member, bookable_item): - client = get_api_client(user=member) - - # Verify the item exists before deletion - exists_before_delete = BookableItem.objects.filter(id=bookable_item.id).exists() - assert ( - exists_before_delete is True - ), "The bookable item should exist in the database before deletion." - - # Perform the deletion request - response = client.delete( - f"/kontres/bookable_items/{bookable_item.id}/", format="json" - ) - - # Check the response status code to ensure the request was processed as expected - assert response.status_code == status.HTTP_200_OK - - # Verify the item does not exist after deletion - exists_after_delete = BookableItem.objects.filter(id=bookable_item.id).exists() - assert ( - exists_after_delete is True - ), "The bookable item should not exist in the database after deletion." diff --git a/app/tests/kontres/test_reservation_integration.py b/app/tests/kontres/test_reservation_integration.py index 809effef..9f651bc1 100644 --- a/app/tests/kontres/test_reservation_integration.py +++ b/app/tests/kontres/test_reservation_integration.py @@ -161,7 +161,8 @@ 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_200_OK + assert response.status_code == status.HTTP_204_NO_CONTENT + assert response.data["detail"] == "Reservasjonen ble slettet." @pytest.mark.django_db @@ -375,7 +376,7 @@ def test_admin_can_delete_any_reservation(admin_user, reservation): f"/kontres/reservations/{reservation.id}/", format="json", ) - assert response.status_code == status.HTTP_200_OK + assert response.status_code == status.HTTP_204_NO_CONTENT @pytest.mark.django_db