From a506458e9e40bd99c425fcf4f58877ed743cdb7c Mon Sep 17 00:00:00 2001 From: Roffenlund Date: Thu, 28 Aug 2025 13:58:55 +0300 Subject: [PATCH 01/61] Refactor and update test utils Add file with endpoints and PATCH/POST data to a separate file in order to not clutter the actual test fiels with this data. Implement helper functions for getting superuser and superuser with a package and other testing setup. Refs. TS-2584 --- .../api/cyberstorm/tests/endpoint_data.py | 70 +++++++++++++++++++ .../api/cyberstorm/tests/utils.py | 65 ++++++++++++++++- 2 files changed, 134 insertions(+), 1 deletion(-) create mode 100644 django/thunderstore/api/cyberstorm/tests/endpoint_data.py diff --git a/django/thunderstore/api/cyberstorm/tests/endpoint_data.py b/django/thunderstore/api/cyberstorm/tests/endpoint_data.py new file mode 100644 index 000000000..17bdf3b96 --- /dev/null +++ b/django/thunderstore/api/cyberstorm/tests/endpoint_data.py @@ -0,0 +1,70 @@ +ENDPOINTS = { + "GET": [ + "/api/cyberstorm/community/", + "/api/cyberstorm/community/{community_id}/", + "/api/cyberstorm/community/{community_id}/filters/", + "/api/cyberstorm/listing/{community_id}/", + "/api/cyberstorm/listing/{community_id}/{namespace_id}/", + "/api/cyberstorm/listing/{community_id}/{namespace_id}/{package_name}/", + "/api/cyberstorm/listing/{community_id}/{namespace_id}/{package_name}/dependants/", + "/api/cyberstorm/package/{community_id}/{namespace_id}/{package_name}/permissions/", + "/api/cyberstorm/package/{namespace_id}/{package_name}/latest/changelog/", + "/api/cyberstorm/package/{namespace_id}/{package_name}/latest/readme/", + "/api/cyberstorm/package/{namespace_id}/{package_name}/v/{version_number}/changelog/", + "/api/cyberstorm/package/{namespace_id}/{package_name}/v/{version_number}/readme/", + "/api/cyberstorm/package/{namespace_id}/{package_name}/versions/", + "/api/cyberstorm/team/{team_id}/", + "/api/cyberstorm/team/{team_id}/member/", + "/api/cyberstorm/team/{team_id}/service-account/", + ], + "POST": { + "/api/cyberstorm/listing/{community_id}/{namespace_id}/{package_name}/approve/": { + "internal_notes": "This is an example internal note", + }, + "/api/cyberstorm/listing/{community_id}/{namespace_id}/{package_name}/reject/": { + "rejection_reason": "This is an example rejection reason", + }, + "/api/cyberstorm/listing/{community_id}/{namespace_id}/{package_name}/update/": { + "categories": ["test"], + }, + "/api/cyberstorm/package/{namespace_id}/{package_name}/deprecate/": { + "deprecate": True, + }, + "/api/cyberstorm/package/{namespace_id}/{package_name}/rate/": { + "target_state": "rated", + }, + "/api/cyberstorm/team/create/": { + "name": "TestTeam", + }, + "/api/cyberstorm/team/{team_name}/member/add/": { + "username": "TestUser", + "role": "member", + }, + }, + "PATCH": { + "/api/cyberstorm/team/{team_name}/update/": { + "donation_link": "https://example.com/donate", + }, + }, + "DELETE": [ + "/api/cyberstorm/team/{team_name}/disband/", + ], +} + + +GET_TEST_CASES = [{"path": path} for path in ENDPOINTS["GET"]] + + +POST_TEST_CASES = [ + {"path": path, "payload": payload} + for path, payload in ENDPOINTS["POST"].items() +] + + +PATCH_TEST_CASES = [ + {"path": path, "payload": payload} + for path, payload in ENDPOINTS["PATCH"].items() +] + + +DELETE_TEST_CASES = [{"path": path} for path in ENDPOINTS["DELETE"]] diff --git a/django/thunderstore/api/cyberstorm/tests/utils.py b/django/thunderstore/api/cyberstorm/tests/utils.py index 7d86ada36..9a5d7f55c 100644 --- a/django/thunderstore/api/cyberstorm/tests/utils.py +++ b/django/thunderstore/api/cyberstorm/tests/utils.py @@ -1,6 +1,41 @@ import re +from typing import Optional +from django.db import connection +from django.test.utils import CaptureQueriesContext from jsonschema import RefResolver, ValidationError, validate +from rest_framework.test import APIClient + +from thunderstore.core.factories import UserFactory +from thunderstore.repository.models import TeamMemberRole + + +def setup_superuser(): + user = UserFactory() + user.is_superuser = True + user.save() + return user + + +def setup_superuser_with_package(package_listing, package_category=None): + user = setup_superuser() + + UserFactory.create(username="TestUser", email="test@user.dev", is_active=True) + + package_listing.package.owner.add_member( + user=user, + role=TeamMemberRole.owner, + ) + + if package_category: + package_category.community = package_listing.community + package_category.save() + + package_listing.package.latest.changelog = "# This is an example changelog" + package_listing.package.latest.readme = "# This is an example readme" + package_listing.package.latest.save() + + return user def convert_x_nullable(schema: dict) -> dict: @@ -109,7 +144,10 @@ def validate_response_against_schema( try: data = response.json() except Exception: - data = response.text + if hasattr(response, "text"): + data = response.text + else: + data = "No response body. Check content-type." errors.append(f"Unexpected status {response.status_code} for {path}: {data}") return errors @@ -155,3 +193,28 @@ def validate_request_body_against_schema( errors.append(error_message) return errors + + +def validate_max_queries( + client: APIClient, + method: str, + path: str, + max_queries: int, + data: Optional[dict] = None, + **kwargs, +): + request_func = getattr(client, method.lower()) + + with CaptureQueriesContext(connection) as ctx: + response = request_func(path, data=data or {}, **kwargs) + + num_queries = len(ctx.captured_queries) + if num_queries > max_queries: + queries_str = "\n".join(q["sql"] for q in ctx.captured_queries) + raise AssertionError( + f"{method} {path} executed {num_queries} queries " + f"(allowed {max_queries}).\n" + f"Queries:\n{queries_str}" + ) + + return response From 60999b6615783b17d16d2ced0130bf6acd9e5611 Mon Sep 17 00:00:00 2001 From: Roffenlund Date: Thu, 28 Aug 2025 14:00:44 +0300 Subject: [PATCH 02/61] Implement query tests and update existing tests Implement new tests for testing the query count for GET endpoints in the cyberstorm API. Implement separate tests for list endpoints to assert they do not exceed the limit of 15 queries. Update the existing endpoint tests accordingly to the refactored changes. Refs. TS-2584 --- .../api/cyberstorm/tests/test_endpoints.py | 258 ++++++++++-------- .../api/cyberstorm/tests/test_query_count.py | 216 +++++++++++++++ 2 files changed, 364 insertions(+), 110 deletions(-) create mode 100644 django/thunderstore/api/cyberstorm/tests/test_query_count.py diff --git a/django/thunderstore/api/cyberstorm/tests/test_endpoints.py b/django/thunderstore/api/cyberstorm/tests/test_endpoints.py index 242efa908..c12411f9f 100644 --- a/django/thunderstore/api/cyberstorm/tests/test_endpoints.py +++ b/django/thunderstore/api/cyberstorm/tests/test_endpoints.py @@ -1,62 +1,28 @@ import pytest +from thunderstore.api.cyberstorm.tests.endpoint_data import ( + DELETE_TEST_CASES, + GET_TEST_CASES, + PATCH_TEST_CASES, + POST_TEST_CASES, +) from thunderstore.api.cyberstorm.tests.utils import ( convert_path_to_schema_style, extract_paths, fill_path_params, get_resolver, get_schema, + setup_superuser_with_package, validate_request_body_against_schema, validate_response_against_schema, ) from thunderstore.api.urls import cyberstorm_urls -from thunderstore.core.factories import UserFactory -from thunderstore.repository.models import PackageListing, TeamMemberRole - -post_payload_map = { - "/api/cyberstorm/listing/{community_id}/{namespace_id}/{package_name}/approve/": { - "internal_notes": "This is an example internal note", - }, - "/api/cyberstorm/listing/{community_id}/{namespace_id}/{package_name}/reject/": { - "rejection_reason": "This is an example rejection reason", - }, - "/api/cyberstorm/listing/{community_id}/{namespace_id}/{package_name}/update/": { - "categories": ["test"], - }, - "/api/cyberstorm/package/{namespace_id}/{package_name}/deprecate/": { - "deprecate": True, - }, - "/api/cyberstorm/package/{namespace_id}/{package_name}/rate/": { - "target_state": "rated" - }, - "/api/cyberstorm/team/create/": { - "name": "TestTeam", - }, - "/api/cyberstorm/team/{team_name}/member/add/": { - "username": "TestUser", - "role": "member", - }, -} - - -put_payload_map = {} - - -patch_payload_map = { - "/api/cyberstorm/team/{team_name}/update/": { - "donation_link": "https://example.com/donate", - } -} - - -payload_mapping = { - "post": post_payload_map, - "put": put_payload_map, - "patch": patch_payload_map, -} +from thunderstore.repository.models import PackageListing def get_parameter_values(package_listing: PackageListing) -> dict: + service_account = package_listing.package.owner.service_accounts.first() + return { "community_id": package_listing.community.identifier, "namespace_id": package_listing.package.owner.get_namespace().name, @@ -64,89 +30,156 @@ def get_parameter_values(package_listing: PackageListing) -> dict: "version_number": package_listing.package.latest.version_number, "team_id": package_listing.package.owner.name, "team_name": package_listing.package.owner.name, + "uuid": service_account.uuid if service_account else "", } -def setup_superuser_with_package(package_listing, package_category=None): - user = UserFactory() - user.is_superuser = True - user.save() +@pytest.mark.django_db +@pytest.mark.parametrize("test_case", GET_TEST_CASES) +def test_cyberstorm_GET_endpoint_schemas( + test_case, api_client, active_package_listing, package_category +): + api_path = test_case["path"] + user = setup_superuser_with_package(active_package_listing, package_category) + api_client.force_authenticate(user) + + param_values = get_parameter_values(active_package_listing) + schema = get_schema(api_client) + resolver = get_resolver(schema) - UserFactory.create(username="TestUser", email="test@user.dev", is_active=True) + url = fill_path_params(api_path, param_values) + response = api_client.get(url, format="json") - package_listing.package.owner.add_member( - user=user, - role=TeamMemberRole.owner, + errors = validate_response_against_schema( + response=response, + path=api_path, + method="get", + schema=schema, + resolver=resolver, ) - if package_category: - package_category.community = package_listing.community - package_category.save() + if errors: + pytest.fail(f"Validation errors for GET {api_path}:\n" + "\n".join(errors)) - package_listing.package.latest.changelog = "# This is an example changelog" - package_listing.package.latest.readme = "# This is an example readme" - package_listing.package.latest.save() - return user +@pytest.mark.django_db +@pytest.mark.parametrize("test_case", DELETE_TEST_CASES) +def test_cyberstorm_DELETE_endpoint_schemas( + test_case, api_client, active_package_listing, package_category, service_account +): + api_path = test_case["path"] + user = setup_superuser_with_package(active_package_listing, package_category) + api_client.force_authenticate(user) + service_account.owner = active_package_listing.package.owner + service_account.save(update_fields=("owner",)) -def make_request(method: str, url: str, api_client, data: dict = {}): - method = method.lower() - client_method = getattr(api_client, method) + param_values = get_parameter_values(active_package_listing) + schema = get_schema(api_client) + resolver = get_resolver(schema) - if method in ["get", "delete"]: - return client_method(url) - else: - return client_method(url, data=data, format="json") + url = fill_path_params(api_path, param_values) + + if "disband" in api_path: + user.teams.first().team.owned_packages.all().delete() + + response = api_client.delete(url, format="json") + + errors = validate_response_against_schema( + response=response, + path=api_path, + method="delete", + schema=schema, + resolver=resolver, + ) + + if errors: + pytest.fail(f"Validation errors for DELETE {api_path}:\n" + "\n".join(errors)) @pytest.mark.django_db -@pytest.mark.parametrize("http_verb", ["get", "delete", "post", "put", "patch"]) -def test_cyberstorm_endpoint_schemas( - api_client, active_package_listing, package_category, http_verb +@pytest.mark.parametrize("test_case", POST_TEST_CASES) +def test_cyberstorm_POST_endpoint_schemas( + test_case, api_client, active_package_listing, package_category ): + api_path = test_case["path"] + payload = test_case["payload"] user = setup_superuser_with_package(active_package_listing, package_category) + api_client.force_authenticate(user) + param_values = get_parameter_values(active_package_listing) schema = get_schema(api_client) resolver = get_resolver(schema) - api_paths = extract_paths(schema, "cyberstorm", http_verb) - matched_payload_map = payload_mapping.get(http_verb) - - request_body = {} failures = [] - for path in api_paths: - url = fill_path_params(path, param_values) - api_client.force_authenticate(user) + request_errors = validate_request_body_against_schema( + request_body=payload, + path=api_path, + method="post", + schema=schema, + resolver=resolver, + ) + + if request_errors: + failures.extend(request_errors) - if matched_payload_map: - request_body = matched_payload_map.get(path, {}) - errors = validate_request_body_against_schema( - request_body=request_body, - path=path, - method=http_verb, - schema=schema, - resolver=resolver, - ) + url = fill_path_params(api_path, param_values) + response = api_client.post(url, data=payload, format="json") - failures.extend(errors) + response_errors = validate_response_against_schema( + response=response, + path=api_path, + method="post", + schema=schema, + resolver=resolver, + ) + + if response_errors: + failures.extend(response_errors) - if "disband" in path: # requires special handling in setup - user.teams.first().team.owned_packages.all().delete() + if failures: + pytest.fail("\n".join(failures)) + + +@pytest.mark.django_db +@pytest.mark.parametrize("test_case", PATCH_TEST_CASES) +def test_cyberstorm_PATCH_endpoint_schemas( + test_case, api_client, active_package_listing, package_category +): + api_path = test_case["path"] + payload = test_case["payload"] + user = setup_superuser_with_package(active_package_listing, package_category) + api_client.force_authenticate(user) - response = make_request( - method=http_verb, url=url, api_client=api_client, data=request_body - ) + param_values = get_parameter_values(active_package_listing) + schema = get_schema(api_client) + resolver = get_resolver(schema) + failures = [] - errors = validate_response_against_schema( - response=response, - path=path, - method=http_verb, - schema=schema, - resolver=resolver, - ) + request_errors = validate_request_body_against_schema( + request_body=payload, + path=api_path, + method="patch", + schema=schema, + resolver=resolver, + ) - failures.extend(errors) + if request_errors: + failures.extend(request_errors) + + url = fill_path_params(api_path, param_values) + response = api_client.patch(url, data=payload, format="json") + + response_errors = validate_response_against_schema( + response=response, + path=api_path, + method="patch", + schema=schema, + resolver=resolver, + ) + + if response_errors: + failures.extend(response_errors) if failures: pytest.fail("\n".join(failures)) @@ -165,16 +198,21 @@ def test_validate_extracted_paths_with_urlpatterns(api_client): @pytest.mark.django_db -@pytest.mark.parametrize("http_verb", ["put", "patch", "post"]) -def test_find_missing_endpoints(api_client, http_verb): - schema = get_schema(api_client) - api_paths = extract_paths(schema, "cyberstorm", http_verb) - failures = [] +def test_find_missing_endpoints(): + tested_paths = { + convert_path_to_schema_style(path["path"]) + for path in GET_TEST_CASES + + POST_TEST_CASES + + PATCH_TEST_CASES + + DELETE_TEST_CASES + } - for path in api_paths: - url = convert_path_to_schema_style(path) - if url not in payload_mapping[http_verb]: - failures.append(f"Missing test coverage for: {url}") + existing_paths = { + convert_path_to_schema_style(f"/api/cyberstorm/{url.pattern}") + for url in cyberstorm_urls + } - if failures: - pytest.fail("\n".join(failures)) + missing = existing_paths - tested_paths + + if missing: + pytest.fail("Missing test coverage for:\n" + "\n".join(sorted(missing))) diff --git a/django/thunderstore/api/cyberstorm/tests/test_query_count.py b/django/thunderstore/api/cyberstorm/tests/test_query_count.py new file mode 100644 index 000000000..acc88471e --- /dev/null +++ b/django/thunderstore/api/cyberstorm/tests/test_query_count.py @@ -0,0 +1,216 @@ +import factory +import pytest + +from thunderstore.account.forms import CreateServiceAccountForm +from thunderstore.api.cyberstorm.tests.endpoint_data import GET_TEST_CASES +from thunderstore.community.models import Community +from thunderstore.core.factories import UserFactory +from thunderstore.repository.factories import PackageFactory, PackageVersionFactory +from thunderstore.repository.models import ( + PackageListing, + Team, + TeamMember, + TeamMemberRole, +) + +from .utils import ( + fill_path_params, + setup_superuser, + setup_superuser_with_package, + validate_max_queries, +) + +MAX_QUERIES = 15 + + +def get_parameter_values(package_listing: PackageListing) -> dict: + service_account = package_listing.package.owner.service_accounts.first() + + return { + "community_id": package_listing.community.identifier, + "namespace_id": package_listing.package.owner.get_namespace().name, + "package_name": package_listing.package.name, + "version_number": package_listing.package.latest.version_number, + "team_id": package_listing.package.owner.name, + "team_name": package_listing.package.owner.name, + "uuid": service_account.uuid if service_account else "", + } + + +@pytest.mark.django_db +@pytest.mark.parametrize("test_case", GET_TEST_CASES) +def test_cyberstorm_api_GET_query_count( + test_case, api_client, active_package_listing, package_category +): + api_path = test_case["path"] + user = setup_superuser_with_package(active_package_listing, package_category) + api_client.force_authenticate(user) + param_values = get_parameter_values(active_package_listing) + url = fill_path_params(api_path, param_values) + + validate_max_queries( + client=api_client, + method="get", + path=url, + max_queries=MAX_QUERIES, + ) + + +@pytest.mark.django_db +def test_cybserstorm_community_list_query_count(api_client): + path = "/api/cyberstorm/community/" + + communities = [] + + for x in range(20): + random_name = f"Community_{x}" + random_id = f"community_{x}" + communities.append(Community(name=random_name, identifier=random_id)) + + Community.objects.bulk_create(communities) + + validate_max_queries( + client=api_client, + method="get", + path=path, + max_queries=MAX_QUERIES, + ) + + +@pytest.mark.django_db +def test_cyberstorm_package_versions_list_query_count(api_client, active_package): + url = "/api/cyberstorm/package/{namespace_id}/{package_name}/versions/" + + PackageVersionFactory.create_batch( + 20, + package=active_package, + name=factory.Sequence(lambda n: f"{active_package.name}_2_0_{n+1}"), + version_number=factory.Sequence(lambda n: f"2.0.{n+1}"), + website_url="https://example.org", + description="Example mod", + readme="# This is an example mod", + changelog="# This is an example changelog", + ) + + user = setup_superuser() + api_client.force_authenticate(user) + path_params = { + "namespace_id": active_package.owner.get_namespace().name, + "package_name": active_package.name, + } + path = fill_path_params(url, path_params) + + validate_max_queries( + client=api_client, + method="get", + path=path, + max_queries=MAX_QUERIES, + ) + + +@pytest.mark.django_db +@pytest.mark.parametrize( + "test_case", + [ + {"path": "/api/cyberstorm/listing/{community_id}/"}, + {"path": "/api/cyberstorm/listing/{community_id}/{namespace_id}/"}, + ], +) +def test_cyberstorm_package_listing_list_query_count(test_case, api_client): + amount = 20 + community = Community.objects.create( + name="Test_Community", identifier="test_community" + ) + team = Team.create(name="Test_Team") + namespace = team.get_namespace() + + packages = PackageFactory.create_batch( + amount, + is_active=True, + is_deprecated=False, + owner=team, + namespace=namespace, + name=factory.Sequence(lambda n: f"Test_Package_{n}"), + ) + + PackageVersionFactory.create_batch( + amount, + package=factory.Iterator(packages), + name=factory.Iterator([pkg.name for pkg in packages]), + is_active=True, + ) + + PackageListing.objects.bulk_create( + [PackageListing(community=community, package=pkg) for pkg in packages] + ) + + api_path = test_case["path"] + user = setup_superuser() + api_client.force_authenticate(user) + path_params = { + "community_id": community.identifier, + "namespace_id": namespace.name, + } + url = fill_path_params(api_path, path_params) + + validate_max_queries( + client=api_client, + method="get", + path=url, + max_queries=MAX_QUERIES, + ) + + +@pytest.mark.django_db +def test_cyberstorm_team_member_list_query_count(api_client): + url = "/api/cyberstorm/team/{team_id}/member/" + user = setup_superuser() + team = Team.create(name="Test_Team") + + users = UserFactory.create_batch(20) + TeamMember.objects.bulk_create( + [ + TeamMember(team=team, user=member_user, role=TeamMemberRole.member) + for member_user in users + ] + ) + + api_client.force_authenticate(user) + url = fill_path_params(url, {"team_id": team.name}) + + validate_max_queries( + client=api_client, + method="get", + path=url, + max_queries=MAX_QUERIES, + ) + + +@pytest.mark.django_db +def test_cyberstorm_team_service_accounts_list_query_count(api_client): + url = "/api/cyberstorm/team/{team_id}/service-account/" + user = setup_superuser() + + team = Team.create(name="Test_Team") + team.add_member(user=user, role=TeamMemberRole.owner) + + for x in range(20): + member_user = UserFactory.create(username=f"TestUser_{x+1}") + team.add_member(user=member_user, role=TeamMemberRole.owner) + form = CreateServiceAccountForm( + user, + data={"team": team, "nickname": f"ServiceAccount_{x}"}, + ) + form.is_valid() + form.save() + + api_client.force_authenticate(user) + path_params = {"team_id": team.name} + url = fill_path_params(url, path_params) + + validate_max_queries( + client=api_client, + method="get", + path=url, + max_queries=MAX_QUERIES, + ) From fa1a4378d067a59130f122c623ddd1cba1be3a70 Mon Sep 17 00:00:00 2001 From: Roffenlund Date: Thu, 28 Aug 2025 14:08:19 +0300 Subject: [PATCH 03/61] Fix styling with black Refs. TS-2584 --- django/thunderstore/api/cyberstorm/tests/endpoint_data.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/django/thunderstore/api/cyberstorm/tests/endpoint_data.py b/django/thunderstore/api/cyberstorm/tests/endpoint_data.py index 17bdf3b96..8c04328a7 100644 --- a/django/thunderstore/api/cyberstorm/tests/endpoint_data.py +++ b/django/thunderstore/api/cyberstorm/tests/endpoint_data.py @@ -56,14 +56,12 @@ POST_TEST_CASES = [ - {"path": path, "payload": payload} - for path, payload in ENDPOINTS["POST"].items() + {"path": path, "payload": payload} for path, payload in ENDPOINTS["POST"].items() ] PATCH_TEST_CASES = [ - {"path": path, "payload": payload} - for path, payload in ENDPOINTS["PATCH"].items() + {"path": path, "payload": payload} for path, payload in ENDPOINTS["PATCH"].items() ] From 59bf19d0d3f434b8a5acb874234cbf702387cbd7 Mon Sep 17 00:00:00 2001 From: Roffenlund Date: Thu, 4 Sep 2025 13:08:32 +0300 Subject: [PATCH 04/61] Refactor test utils Remove setup_superuser and use the UserFactory directly. Move get_parameter_values to utils instead of implementing it twice. Fix typo in test. Refs. TS-2597 --- .../api/cyberstorm/tests/test_endpoints.py | 16 +---------- .../api/cyberstorm/tests/test_query_count.py | 28 +++++-------------- .../api/cyberstorm/tests/utils.py | 21 +++++++++----- 3 files changed, 22 insertions(+), 43 deletions(-) diff --git a/django/thunderstore/api/cyberstorm/tests/test_endpoints.py b/django/thunderstore/api/cyberstorm/tests/test_endpoints.py index c12411f9f..674015a4b 100644 --- a/django/thunderstore/api/cyberstorm/tests/test_endpoints.py +++ b/django/thunderstore/api/cyberstorm/tests/test_endpoints.py @@ -10,6 +10,7 @@ convert_path_to_schema_style, extract_paths, fill_path_params, + get_parameter_values, get_resolver, get_schema, setup_superuser_with_package, @@ -17,21 +18,6 @@ validate_response_against_schema, ) from thunderstore.api.urls import cyberstorm_urls -from thunderstore.repository.models import PackageListing - - -def get_parameter_values(package_listing: PackageListing) -> dict: - service_account = package_listing.package.owner.service_accounts.first() - - return { - "community_id": package_listing.community.identifier, - "namespace_id": package_listing.package.owner.get_namespace().name, - "package_name": package_listing.package.name, - "version_number": package_listing.package.latest.version_number, - "team_id": package_listing.package.owner.name, - "team_name": package_listing.package.owner.name, - "uuid": service_account.uuid if service_account else "", - } @pytest.mark.django_db diff --git a/django/thunderstore/api/cyberstorm/tests/test_query_count.py b/django/thunderstore/api/cyberstorm/tests/test_query_count.py index acc88471e..9609d665d 100644 --- a/django/thunderstore/api/cyberstorm/tests/test_query_count.py +++ b/django/thunderstore/api/cyberstorm/tests/test_query_count.py @@ -15,7 +15,7 @@ from .utils import ( fill_path_params, - setup_superuser, + get_parameter_values, setup_superuser_with_package, validate_max_queries, ) @@ -23,20 +23,6 @@ MAX_QUERIES = 15 -def get_parameter_values(package_listing: PackageListing) -> dict: - service_account = package_listing.package.owner.service_accounts.first() - - return { - "community_id": package_listing.community.identifier, - "namespace_id": package_listing.package.owner.get_namespace().name, - "package_name": package_listing.package.name, - "version_number": package_listing.package.latest.version_number, - "team_id": package_listing.package.owner.name, - "team_name": package_listing.package.owner.name, - "uuid": service_account.uuid if service_account else "", - } - - @pytest.mark.django_db @pytest.mark.parametrize("test_case", GET_TEST_CASES) def test_cyberstorm_api_GET_query_count( @@ -57,7 +43,7 @@ def test_cyberstorm_api_GET_query_count( @pytest.mark.django_db -def test_cybserstorm_community_list_query_count(api_client): +def test_cyberstorm_community_list_query_count(api_client): path = "/api/cyberstorm/community/" communities = [] @@ -92,7 +78,7 @@ def test_cyberstorm_package_versions_list_query_count(api_client, active_package changelog="# This is an example changelog", ) - user = setup_superuser() + user = UserFactory.create(is_superuser=True) api_client.force_authenticate(user) path_params = { "namespace_id": active_package.owner.get_namespace().name, @@ -145,7 +131,7 @@ def test_cyberstorm_package_listing_list_query_count(test_case, api_client): ) api_path = test_case["path"] - user = setup_superuser() + user = UserFactory.create(is_superuser=True) api_client.force_authenticate(user) path_params = { "community_id": community.identifier, @@ -164,7 +150,7 @@ def test_cyberstorm_package_listing_list_query_count(test_case, api_client): @pytest.mark.django_db def test_cyberstorm_team_member_list_query_count(api_client): url = "/api/cyberstorm/team/{team_id}/member/" - user = setup_superuser() + super_user = UserFactory.create(is_superuser=True) team = Team.create(name="Test_Team") users = UserFactory.create_batch(20) @@ -175,7 +161,7 @@ def test_cyberstorm_team_member_list_query_count(api_client): ] ) - api_client.force_authenticate(user) + api_client.force_authenticate(super_user) url = fill_path_params(url, {"team_id": team.name}) validate_max_queries( @@ -189,7 +175,7 @@ def test_cyberstorm_team_member_list_query_count(api_client): @pytest.mark.django_db def test_cyberstorm_team_service_accounts_list_query_count(api_client): url = "/api/cyberstorm/team/{team_id}/service-account/" - user = setup_superuser() + user = UserFactory.create(is_superuser=True) team = Team.create(name="Test_Team") team.add_member(user=user, role=TeamMemberRole.owner) diff --git a/django/thunderstore/api/cyberstorm/tests/utils.py b/django/thunderstore/api/cyberstorm/tests/utils.py index 9a5d7f55c..8ddbb9cb9 100644 --- a/django/thunderstore/api/cyberstorm/tests/utils.py +++ b/django/thunderstore/api/cyberstorm/tests/utils.py @@ -7,18 +7,25 @@ from rest_framework.test import APIClient from thunderstore.core.factories import UserFactory -from thunderstore.repository.models import TeamMemberRole +from thunderstore.repository.models import PackageListing, TeamMemberRole -def setup_superuser(): - user = UserFactory() - user.is_superuser = True - user.save() - return user +def get_parameter_values(package_listing: PackageListing) -> dict: + service_account = package_listing.package.owner.service_accounts.first() + + return { + "community_id": package_listing.community.identifier, + "namespace_id": package_listing.package.owner.get_namespace().name, + "package_name": package_listing.package.name, + "version_number": package_listing.package.latest.version_number, + "team_id": package_listing.package.owner.name, + "team_name": package_listing.package.owner.name, + "uuid": service_account.uuid if service_account else "", + } def setup_superuser_with_package(package_listing, package_category=None): - user = setup_superuser() + user = UserFactory.create(is_superuser=True) UserFactory.create(username="TestUser", email="test@user.dev", is_active=True) From 9843753e9901320ea63692a539b95d368744b81e Mon Sep 17 00:00:00 2001 From: Roffenlund Date: Mon, 22 Sep 2025 10:58:59 +0300 Subject: [PATCH 05/61] Improve get_package_version Add "latest" field to select_related in order to slightly improve database query when fetching latest package version. Refs. TS-2639 --- django/thunderstore/api/cyberstorm/views/markdown.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/django/thunderstore/api/cyberstorm/views/markdown.py b/django/thunderstore/api/cyberstorm/views/markdown.py index 548940615..5c93d54ec 100644 --- a/django/thunderstore/api/cyberstorm/views/markdown.py +++ b/django/thunderstore/api/cyberstorm/views/markdown.py @@ -60,7 +60,7 @@ def get_package_version( version_number: Optional[str], ) -> PackageVersion: package = get_object_or_404( - Package.objects.active(), + Package.objects.active().select_related("latest"), namespace__name=namespace_id, name=package_name, ) From 6731f288ad9a048dbef20e094dc87b04d7a67a03 Mon Sep 17 00:00:00 2001 From: Roffenlund Date: Mon, 22 Sep 2025 11:01:37 +0300 Subject: [PATCH 06/61] Implement pagination.py file in cyberstorm API Add a new file for storing pagination classes as these ought to be in one place. Implement a new class for package dependencies list apiview. Refs. TS-2639 --- django/thunderstore/api/pagination.py | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 django/thunderstore/api/pagination.py diff --git a/django/thunderstore/api/pagination.py b/django/thunderstore/api/pagination.py new file mode 100644 index 000000000..3d38ce950 --- /dev/null +++ b/django/thunderstore/api/pagination.py @@ -0,0 +1,5 @@ +from rest_framework.pagination import PageNumberPagination + + +class PackageDependenciesPaginator(PageNumberPagination): + page_size = 20 From adb6766cfaedffdb8f3794a6ec00eff517ed24b0 Mon Sep 17 00:00:00 2001 From: Roffenlund Date: Mon, 22 Sep 2025 11:02:59 +0300 Subject: [PATCH 07/61] Add dependencies serializer Add new serializer to be used with the package dependencies list apiview. Refs. TS-2639 --- .../api/cyberstorm/serializers/__init__.py | 7 +++++- .../api/cyberstorm/serializers/package.py | 22 +++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/django/thunderstore/api/cyberstorm/serializers/__init__.py b/django/thunderstore/api/cyberstorm/serializers/__init__.py index f09d2bab5..e59a52700 100644 --- a/django/thunderstore/api/cyberstorm/serializers/__init__.py +++ b/django/thunderstore/api/cyberstorm/serializers/__init__.py @@ -3,7 +3,11 @@ CyberstormPackageCategorySerializer, CyberstormPackageListingSectionSerializer, ) -from .package import CyberstormPackagePreviewSerializer, PackagePermissionsSerializer +from .package import ( + CyberstormPackageDependencySerializer, + CyberstormPackagePreviewSerializer, + PackagePermissionsSerializer, +) from .team import ( CyberstormCreateTeamSerializer, CyberstormServiceAccountSerializer, @@ -27,4 +31,5 @@ "CyberstormTeamSerializer", "PackagePermissionsSerializer", "CyberstormTeamUpdateSerializer", + "CyberstormPackageDependencySerializer", ] diff --git a/django/thunderstore/api/cyberstorm/serializers/package.py b/django/thunderstore/api/cyberstorm/serializers/package.py index f0d5c8674..000c325a7 100644 --- a/django/thunderstore/api/cyberstorm/serializers/package.py +++ b/django/thunderstore/api/cyberstorm/serializers/package.py @@ -1,3 +1,5 @@ +from typing import Optional + from rest_framework import serializers from thunderstore.api.cyberstorm.serializers.community import ( @@ -55,3 +57,23 @@ class CyberstormPackagePreviewSerializer(serializers.Serializer): rating_count = serializers.IntegerField(min_value=0) size = serializers.IntegerField(min_value=0) datetime_created = serializers.DateTimeField() + + +class CyberstormPackageDependencySerializer(serializers.Serializer): + description = serializers.SerializerMethodField() + icon_url = serializers.SerializerMethodField() + is_active = serializers.BooleanField(source="is_effectively_active") + name = serializers.CharField() + namespace = serializers.CharField(source="package.namespace.name") + version_number = serializers.CharField() + is_removed = serializers.BooleanField() + + def get_description(self, obj: PackageVersion) -> str: + return ( + obj.description + if obj.is_effectively_active + else "This package has been removed." + ) + + def get_icon_url(self, obj: PackageVersion) -> Optional[str]: + return obj.icon.url if obj.is_effectively_active else None From 00cefeed67638cb2018427282d09a93397db2c19 Mon Sep 17 00:00:00 2001 From: Roffenlund Date: Mon, 22 Sep 2025 11:03:51 +0300 Subject: [PATCH 08/61] Implement package dependencies list api view Implement a view responsible for fetching dependencies for a package. The view returns a list of paginated and serialized package versions related to a specific package. Pagination is set to 20 items per page. The view expects a version number, or alternatively "latest" in the URL kwargs to be present. When using "latest", the version is set to None which is handled by get_package_version(). Refs. TS-2639 --- .../api/cyberstorm/views/__init__.py | 6 +++- .../cyberstorm/views/package_version_list.py | 32 ++++++++++++++++++- 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/django/thunderstore/api/cyberstorm/views/__init__.py b/django/thunderstore/api/cyberstorm/views/__init__.py index 86324482b..88b02bbdc 100644 --- a/django/thunderstore/api/cyberstorm/views/__init__.py +++ b/django/thunderstore/api/cyberstorm/views/__init__.py @@ -16,7 +16,10 @@ ) from .package_permissions import PackagePermissionsAPIView from .package_rating import RatePackageAPIView -from .package_version_list import PackageVersionListAPIView +from .package_version_list import ( + PackageVersionDependenciesListAPIView, + PackageVersionListAPIView, +) from .team import ( DisbandTeamAPIView, TeamAPIView, @@ -39,6 +42,7 @@ "PackageListingByNamespaceListAPIView", "PackagePermissionsAPIView", "PackageVersionChangelogAPIView", + "PackageVersionDependenciesListAPIView", "PackageVersionListAPIView", "PackageVersionReadmeAPIView", "TeamAPIView", diff --git a/django/thunderstore/api/cyberstorm/views/package_version_list.py b/django/thunderstore/api/cyberstorm/views/package_version_list.py index 51c60c7b9..927e06fbd 100644 --- a/django/thunderstore/api/cyberstorm/views/package_version_list.py +++ b/django/thunderstore/api/cyberstorm/views/package_version_list.py @@ -1,8 +1,14 @@ +from django.db.models import QuerySet from rest_framework import serializers from rest_framework.generics import ListAPIView, get_object_or_404 +from thunderstore.api.cyberstorm.serializers import ( + CyberstormPackageDependencySerializer, +) +from thunderstore.api.cyberstorm.views.markdown import get_package_version +from thunderstore.api.pagination import PackageDependenciesPaginator from thunderstore.api.utils import CyberstormAutoSchemaMixin -from thunderstore.repository.models import Package +from thunderstore.repository.models import Package, PackageVersion class CyberstormPackageVersionSerializer(serializers.Serializer): @@ -28,3 +34,27 @@ def get_queryset(self): ) return package.versions.active() + + +class PackageVersionDependenciesListAPIView(CyberstormAutoSchemaMixin, ListAPIView): + serializer_class = CyberstormPackageDependencySerializer + pagination_class = PackageDependenciesPaginator + + def get_queryset(self) -> QuerySet[PackageVersion]: + version_number = self.kwargs.get("version_number") + if version_number == "latest": + version_number = None # get_package_version understands None as latest + + package_version = get_package_version( + namespace_id=self.kwargs["namespace_id"], + package_name=self.kwargs["package_name"], + version_number=version_number, + ) + + qs = ( + package_version.dependencies.all() + .select_related("package", "package__namespace") + .order_by("package__namespace__name", "package__name") + ) + + return qs From 562a3b4afbe27216e91caa5aae5edd17bb1e6712 Mon Sep 17 00:00:00 2001 From: Roffenlund Date: Mon, 22 Sep 2025 11:07:49 +0300 Subject: [PATCH 09/61] Add URL for package dependencies api list view Add URL and tests for package dependencies API list view. Refs. TS-2639 --- .../tests/test_package_version_list.py | 112 +++++++++++++++++- django/thunderstore/api/urls.py | 6 + 2 files changed, 117 insertions(+), 1 deletion(-) diff --git a/django/thunderstore/api/cyberstorm/tests/test_package_version_list.py b/django/thunderstore/api/cyberstorm/tests/test_package_version_list.py index e8bd57d9f..bd399cb08 100644 --- a/django/thunderstore/api/cyberstorm/tests/test_package_version_list.py +++ b/django/thunderstore/api/cyberstorm/tests/test_package_version_list.py @@ -1,7 +1,11 @@ +from typing import Optional + import pytest +from django.db import connection +from django.test.utils import CaptureQueriesContext from rest_framework.test import APIClient -from thunderstore.repository.factories import PackageVersionFactory +from thunderstore.repository.factories import PackageFactory, PackageVersionFactory from thunderstore.repository.models import Package @@ -48,3 +52,109 @@ def test_package_version_list_api_view__returns_versions( assert len(actual) == 1 assert actual[0]["version_number"] == expected.version_number + + +@pytest.mark.django_db +@pytest.mark.parametrize( + "version, should_raise_404", + [ + ("1.0.0", False), + ("latest", False), + ("hello", True), + ("world", True), + (None, True), + ("", True), + ], +) +def test_package_version_dependencies_list( + api_client: APIClient, + version: Optional[str], + should_raise_404: bool, +) -> None: + """ "Test the PackageVersionDependenciesListAPIView with different version inputs.""" + + dependency_count = 1 + + package = PackageFactory(name="TestPackage") + PackageVersionFactory(package=package) + + target_dependencies = [PackageVersionFactory() for _ in range(dependency_count)] + package.latest.dependencies.set([dep.id for dep in target_dependencies]) + + namespace = package.namespace.name + package_name = package.name + + url = ( + f"/api/cyberstorm/package/{namespace}/{package_name}/v/{version}/dependencies/" + ) + response = api_client.get(url) + + if should_raise_404: + assert response.status_code == 404 + else: + assert response.status_code == 200 + assert response.json()["count"] == dependency_count + + +@pytest.mark.django_db +def test_package_version_dependencies_query_count(api_client: APIClient) -> None: + """Ensure that the number of database queries remains low regardless with 100 dependencies.""" + + dependency_count = 100 + query_count = 25 + + package = PackageFactory(name="TestPackage") + PackageVersionFactory(package=package) + + target_dependencies = [PackageVersionFactory() for _ in range(dependency_count)] + package.latest.dependencies.set([dep.id for dep in target_dependencies]) + + namespace = package.namespace.name + package_name = package.name + url = f"/api/cyberstorm/package/{namespace}/{package_name}/v/latest/dependencies/" + + with CaptureQueriesContext(connection) as ctx: + response = api_client.get(url) + + assert response.status_code == 200 + assert len(ctx.captured_queries) <= query_count + + +@pytest.mark.django_db +def test_package_version_dependencies_list_response(api_client: APIClient) -> None: + """Test the response structure of the PackageVersionDependenciesListAPIView.""" + + dependency_count = 1 + + package = PackageFactory(name="TestPackage") + PackageVersionFactory(package=package) + + target_dependencies = [PackageVersionFactory() for _ in range(dependency_count)] + package.latest.dependencies.set([dep.id for dep in target_dependencies]) + + namespace = package.namespace.name + package_name = package.name + + url = f"/api/cyberstorm/package/{namespace}/{package_name}/v/latest/dependencies/" + response = api_client.get(url) + + target_version = target_dependencies[0] + expected_data = { + "count": 1, + "next": None, + "previous": None, + "results": [ + { + "description": target_version.description, + "icon_url": target_version.icon.url, + "is_active": True, + "name": target_version.name, + "namespace": target_version.package.namespace.name, + "version_number": target_version.version_number, + "is_removed": False, + } + ], + } + + assert response.status_code == 200 + assert response.json() == expected_data diff --git a/django/thunderstore/api/urls.py b/django/thunderstore/api/urls.py index dc35c1471..834c72188 100644 --- a/django/thunderstore/api/urls.py +++ b/django/thunderstore/api/urls.py @@ -13,6 +13,7 @@ PackageListingByNamespaceListAPIView, PackagePermissionsAPIView, PackageVersionChangelogAPIView, + PackageVersionDependenciesListAPIView, PackageVersionListAPIView, PackageVersionReadmeAPIView, RatePackageAPIView, @@ -97,6 +98,11 @@ PackageVersionReadmeAPIView.as_view(), name="cyberstorm.package.version.readme", ), + path( + "package///v//dependencies/", + PackageVersionDependenciesListAPIView.as_view(), + name="cyberstorm.package.version.dependencies-list", + ), path( "package///versions/", PackageVersionListAPIView.as_view(), From 631291b8cc8ff6907e875696cf49868c741ade3f Mon Sep 17 00:00:00 2001 From: Roffenlund Date: Wed, 17 Sep 2025 12:43:09 +0300 Subject: [PATCH 10/61] Implement service for remove team member Refs. TS-2637 --- .../api/cyberstorm/services/team.py | 10 ++- .../tests/services/test_team_services.py | 65 +++++++++++++++++++ 2 files changed, 74 insertions(+), 1 deletion(-) diff --git a/django/thunderstore/api/cyberstorm/services/team.py b/django/thunderstore/api/cyberstorm/services/team.py index 4ea78033c..09043ff39 100644 --- a/django/thunderstore/api/cyberstorm/services/team.py +++ b/django/thunderstore/api/cyberstorm/services/team.py @@ -5,7 +5,7 @@ from thunderstore.core.exceptions import PermissionValidationError from thunderstore.core.types import UserType from thunderstore.repository.models import Namespace, Team -from thunderstore.repository.models.team import TeamMemberRole +from thunderstore.repository.models.team import TeamMember, TeamMemberRole @transaction.atomic @@ -42,3 +42,11 @@ def update_team(agent: UserType, team: Team, donation_link: str) -> Team: team.save() return team + + +@transaction.atomic +def remove_team_member(agent: UserType, member: TeamMember) -> None: + if member.user != agent: + member.team.ensure_user_can_manage_members(agent) + member.team.ensure_member_can_be_removed(member) + member.delete() diff --git a/django/thunderstore/api/cyberstorm/tests/services/test_team_services.py b/django/thunderstore/api/cyberstorm/tests/services/test_team_services.py index fd210d37d..2a8b7a05f 100644 --- a/django/thunderstore/api/cyberstorm/tests/services/test_team_services.py +++ b/django/thunderstore/api/cyberstorm/tests/services/test_team_services.py @@ -2,9 +2,11 @@ from django.core.exceptions import ValidationError from django.http import Http404 +from conftest import TestUserTypes from thunderstore.api.cyberstorm.services import team as team_services from thunderstore.core.exceptions import PermissionValidationError from thunderstore.repository.models import Namespace, Team, TeamMemberRole +from thunderstore.repository.models.team import TeamMember @pytest.mark.django_db @@ -103,3 +105,66 @@ def test_update_team_user_cannot_edit_info(team_member): team=team_member.team, donation_link=new_donation_link, ) + + +@pytest.mark.django_db +def test_remove_team_member_success(team_owner, user): + """Owner removes another member successfully.""" + + team = team_owner.team + team.add_member(user=user, role=TeamMemberRole.member) + + member = TeamMember.objects.get(team=team_owner.team, user=user) + team_services.remove_team_member(agent=team_owner.user, member=member) + + assert not TeamMember.objects.filter(pk=member.pk).exists() + + +@pytest.mark.django_db +def test_remove_team_member_remove_self(team_member): + """A member removes themselves from the team successfully.""" + + member = TeamMember.objects.get(team=team_member.team, user=team_member.user) + team_services.remove_team_member(agent=team_member.user, member=member) + + assert not TeamMember.objects.filter(pk=member.pk).exists() + + +@pytest.mark.django_db +def test_remove_team_member_user_cannot_manage_members(user, team_owner): + """A non-owner user tries to remove another member.""" + + team = team_owner.team + team.add_member(user=user, role=TeamMemberRole.member) + member = TeamMember.objects.get(team=team_owner.team, user=team_owner.user) + + error_msg = "Must be an owner to manage team members" + with pytest.raises(PermissionValidationError, match=error_msg): + team_services.remove_team_member(agent=user, member=member) + + +@pytest.mark.django_db +def test_remove_team_member_cannot_remove_last_owner(team_owner): + """Owner tries to remove themselves when they are the last owner.""" + + team = team_owner.team + member = TeamMember.objects.get(team=team, user=team_owner.user) + + error_msg = "Cannot remove last owner from team" + with pytest.raises(PermissionValidationError, match=error_msg): + team_services.remove_team_member(agent=team_owner.user, member=member) + + +@pytest.mark.django_db +def test_remove_team_member_from_another_team(team_owner, user): + """An owner tries to remove a member from another team.""" + + another_team = Team.objects.create(name="AnotherTeam") + another_team.add_member(user=user, role=TeamMemberRole.member) + another_team_member = TeamMember.objects.get(team=another_team, user=user) + + error_msg = "Must be an owner to manage team members" + with pytest.raises(PermissionValidationError, match=error_msg): + team_services.remove_team_member( + agent=team_owner.user, member=another_team_member + ) From d3fe9c3bdaca1c95f24cfb362fcc6636720e6c97 Mon Sep 17 00:00:00 2001 From: Roffenlund Date: Wed, 17 Sep 2025 12:44:07 +0300 Subject: [PATCH 11/61] Implement api view for removing team member Implement a DELETE endpoint for removing team members from a team in the cyberstorm API. Implement URL and tests. Refs. TS-2637 --- .../api/cyberstorm/tests/test_team.py | 105 ++++++++++++++++++ .../api/cyberstorm/views/__init__.py | 2 + .../thunderstore/api/cyberstorm/views/team.py | 23 ++++ django/thunderstore/api/urls.py | 6 + 4 files changed, 136 insertions(+) diff --git a/django/thunderstore/api/cyberstorm/tests/test_team.py b/django/thunderstore/api/cyberstorm/tests/test_team.py index d07dc2fb1..c435e626f 100644 --- a/django/thunderstore/api/cyberstorm/tests/test_team.py +++ b/django/thunderstore/api/cyberstorm/tests/test_team.py @@ -2,6 +2,7 @@ import pytest from django.contrib.auth import get_user_model +from rest_framework import status from rest_framework.test import APIClient from thunderstore.account.factories import ServiceAccountFactory @@ -456,3 +457,107 @@ def test_team_update_fail_user_not_team_member( assert response.status_code == 403 assert response.json() == expected_response + + +@pytest.mark.django_db +def test_remove_team_member_succeeds( + api_client: APIClient, + team_owner: UserType, + user: UserType, +): + """An owner removes another member successfully.""" + + team = team_owner.team + team.add_member(user=user, role="member") + api_client.force_authenticate(team_owner.user) + + url = f"/api/cyberstorm/team/{team.name}/member/{user.username}/remove/" + response = api_client.delete(url, content_type="application/json") + + assert response.status_code == status.HTTP_204_NO_CONTENT + assert not team.members.filter(user=user).exists() + + +@pytest.mark.django_db +def test_remove_team_member_remove_self(api_client, team_member): + """A member removes themselves successfully.""" + + team = team_member.team + api_client.force_authenticate(team_member.user) + + url = f"/api/cyberstorm/team/{team.name}/member/{team_member.user.username}/remove/" + response = api_client.delete(url) + + assert response.status_code == status.HTTP_204_NO_CONTENT + assert not team.members.filter(user=team_member.user).exists() + + +@pytest.mark.django_db +def test_remove_team_member_user_cannot_manage_members(api_client, team_owner, user): + """Non-owner user tries to remove another member.""" + + team = team_owner.team + team.add_member(user=user, role="member") + api_client.force_authenticate(user) + + url = f"/api/cyberstorm/team/{team.name}/member/{team_owner.user.username}/remove/" + response = api_client.delete(url) + + assert response.status_code == status.HTTP_403_FORBIDDEN + assert team.members.filter(user=team_owner.user).exists() + + +@pytest.mark.django_db +def test_remove_team_member_cannot_remove_last_owner(api_client, team_owner): + """Owner tries to remove themselves when they are the last owner.""" + + team = team_owner.team + api_client.force_authenticate(team_owner.user) + + url = f"/api/cyberstorm/team/{team.name}/member/{team_owner.user.username}/remove/" + response = api_client.delete(url) + + assert response.status_code == status.HTTP_403_FORBIDDEN + assert team.members.filter(user=team_owner.user).exists() + + +@pytest.mark.django_db +def test_remove_team_member_from_another_team(api_client, team_owner, user): + """An owner tries to remove a member from another team.""" + + another_team = Team.objects.create(name="AnotherTeam") + another_team.add_member(user=user, role="member") + api_client.force_authenticate(team_owner.user) + + url = f"/api/cyberstorm/team/{another_team.name}/member/{user.username}/remove/" + response = api_client.delete(url) + + assert response.status_code == status.HTTP_403_FORBIDDEN + assert another_team.members.filter(user=user).exists() + + +@pytest.mark.django_db +def test_remove_team_member_nonexistent_team(api_client, team_owner, team_member): + """Removing a member from a team that does not exist.""" + + api_client.force_authenticate(team_owner.user) + + url = ( + f"/api/cyberstorm/team/doesnotexist/member/{team_member.user.username}/remove/" + ) + response = api_client.delete(url) + + assert response.status_code == status.HTTP_404_NOT_FOUND + + +@pytest.mark.django_db +def test_remove_team_member_nonexistent_member(api_client, team_owner): + """Removing a member who does not exist in the team.""" + + team = team_owner.team + api_client.force_authenticate(team_owner.user) + + url = f"/api/cyberstorm/team/{team.name}/member/notarealuser/remove/" + response = api_client.delete(url) + + assert response.status_code == status.HTTP_404_NOT_FOUND diff --git a/django/thunderstore/api/cyberstorm/views/__init__.py b/django/thunderstore/api/cyberstorm/views/__init__.py index 86324482b..167a35764 100644 --- a/django/thunderstore/api/cyberstorm/views/__init__.py +++ b/django/thunderstore/api/cyberstorm/views/__init__.py @@ -23,6 +23,7 @@ TeamCreateAPIView, TeamMemberAddAPIView, TeamMemberListAPIView, + TeamMemberRemoveAPIView, TeamServiceAccountListAPIView, UpdateTeamAPIView, ) @@ -43,6 +44,7 @@ "PackageVersionReadmeAPIView", "TeamAPIView", "TeamCreateAPIView", + "TeamMemberRemoveAPIView", "TeamMemberAddAPIView", "TeamMemberListAPIView", "TeamServiceAccountListAPIView", diff --git a/django/thunderstore/api/cyberstorm/views/team.py b/django/thunderstore/api/cyberstorm/views/team.py index e360b58cd..0c6bcea1d 100644 --- a/django/thunderstore/api/cyberstorm/views/team.py +++ b/django/thunderstore/api/cyberstorm/views/team.py @@ -20,6 +20,7 @@ from thunderstore.api.cyberstorm.services.team import ( create_team, disband_team, + remove_team_member, update_team, ) from thunderstore.api.ordering import StrictOrderingFilter @@ -118,6 +119,28 @@ def post(self, request, team_name, format=None): raise ValidationError(form.errors) +class TeamMemberRemoveAPIView(APIView): + permission_classes = [IsAuthenticated] + + @conditional_swagger_auto_schema( + request_body=None, + responses={204: ""}, + operation_id="cyberstorm.team.member.remove", + tags=["cyberstorm"], + ) + def delete(self, request, team_name, username): + team = get_object_or_404(Team, name=team_name) + member = get_object_or_404( + TeamMember.objects.real_users().select_related("user"), + team=team, + user__username=username, + ) + + remove_team_member(agent=request.user, member=member) + + return Response(status=status.HTTP_204_NO_CONTENT) + + class TeamServiceAccountListAPIView(CyberstormAutoSchemaMixin, TeamRestrictedAPIView): serializer_class = CyberstormServiceAccountSerializer filter_backends = [StrictOrderingFilter] diff --git a/django/thunderstore/api/urls.py b/django/thunderstore/api/urls.py index dc35c1471..caa8978e9 100644 --- a/django/thunderstore/api/urls.py +++ b/django/thunderstore/api/urls.py @@ -21,6 +21,7 @@ TeamCreateAPIView, TeamMemberAddAPIView, TeamMemberListAPIView, + TeamMemberRemoveAPIView, TeamServiceAccountListAPIView, UpdatePackageListingCategoriesAPIView, UpdateTeamAPIView, @@ -147,6 +148,11 @@ TeamMemberAddAPIView.as_view(), name="cyberstorm.team.member.add", ), + path( + "team//member//remove/", + TeamMemberRemoveAPIView.as_view(), + name="cyberstorm.team.member.remove", + ), path( "team//service-account/", TeamServiceAccountListAPIView.as_view(), From 7848f5a0c320a0456c413cfcb9c2cb7f75a56868 Mon Sep 17 00:00:00 2001 From: Roffenlund Date: Wed, 17 Sep 2025 12:45:13 +0300 Subject: [PATCH 12/61] Update existing form and form view Update the existing form and form view responsible for removing team members from a team to use the service layer function. Update tests accordingly. Refs. TS-2637 --- django/thunderstore/repository/forms/team.py | 20 ++++++++++--------- django/thunderstore/repository/models/team.py | 4 ++-- .../repository/tests/test_team_forms.py | 4 ++++ .../repository/views/team_settings.py | 11 +++++----- 4 files changed, 23 insertions(+), 16 deletions(-) diff --git a/django/thunderstore/repository/forms/team.py b/django/thunderstore/repository/forms/team.py index b03022cfc..15b98d9cf 100644 --- a/django/thunderstore/repository/forms/team.py +++ b/django/thunderstore/repository/forms/team.py @@ -4,7 +4,7 @@ from django.contrib.auth import get_user_model from django.core.exceptions import ObjectDoesNotExist, ValidationError -from thunderstore.api.cyberstorm.services.team import update_team +from thunderstore.api.cyberstorm.services.team import remove_team_member, update_team from thunderstore.core.exceptions import PermissionValidationError from thunderstore.core.types import UserType from thunderstore.repository.models import ( @@ -94,15 +94,17 @@ def __init__(self, user: Optional[UserType], *args, **kwargs): super().__init__(*args, **kwargs) self.user = user - def clean_membership(self): - membership = self.cleaned_data["membership"] - if membership.user != self.user: - membership.team.ensure_user_can_manage_members(self.user) - membership.team.ensure_member_can_be_removed(membership) - return membership - def save(self): - self.cleaned_data["membership"].delete() + if self.errors: + raise ValidationError(self.errors) + + member = self.cleaned_data["membership"] + + try: + remove_team_member(agent=self.user, member=member) + except ValidationError as e: + self.add_error(None, e) + raise ValidationError(self.errors) class EditTeamMemberForm(forms.ModelForm): diff --git a/django/thunderstore/repository/models/team.py b/django/thunderstore/repository/models/team.py index a77782a0f..8bff5acb8 100644 --- a/django/thunderstore/repository/models/team.py +++ b/django/thunderstore/repository/models/team.py @@ -318,9 +318,9 @@ def ensure_member_can_be_removed(self, member: Optional[TeamMember]) -> None: if not member: raise ValidationError("Invalid member") if member.team != self: - raise ValidationError("Member is not a part of this team") + raise PermissionValidationError("Member is not a part of this team") if self.is_last_owner(member): - raise ValidationError("Cannot remove last owner from team") + raise PermissionValidationError("Cannot remove last owner from team") def ensure_member_role_can_be_changed( self, member: Optional[TeamMember], new_role: Optional[str] diff --git a/django/thunderstore/repository/tests/test_team_forms.py b/django/thunderstore/repository/tests/test_team_forms.py index 545aed8e2..944e280ec 100644 --- a/django/thunderstore/repository/tests/test_team_forms.py +++ b/django/thunderstore/repository/tests/test_team_forms.py @@ -254,6 +254,8 @@ def test_form_remove_team_member( assert form.save() is None assert TeamMember.objects.filter(pk=membership).exists() is False else: + with pytest.raises(ValidationError): + form.save() assert form.is_valid() is False assert form.errors @@ -295,6 +297,8 @@ def test_form_remove_team_member_last_owner() -> None: user=user, data={"membership": last_owner.pk}, ) + with pytest.raises(ValidationError): + form.save() assert form.is_valid() is False assert "Cannot remove last owner from team" in str(repr(form.errors)) diff --git a/django/thunderstore/repository/views/team_settings.py b/django/thunderstore/repository/views/team_settings.py index 69237170e..3bde18bb8 100644 --- a/django/thunderstore/repository/views/team_settings.py +++ b/django/thunderstore/repository/views/team_settings.py @@ -16,7 +16,6 @@ CreateServiceAccountForm, DeleteServiceAccountForm, ) -from thunderstore.api.cyberstorm.services.team import update_team from thunderstore.core.mixins import RequireAuthenticationMixin from thunderstore.core.utils import capture_exception from thunderstore.frontend.views import SettingsViewMixin @@ -191,15 +190,17 @@ def get_context_data(self, **kwargs): return context def form_invalid(self, form): - messages.error( - self.request, "There was a problem performing the requested action" - ) + error_msg = "There was a problem performing the requested action" + messages.error(self.request, error_msg) capture_exception(ValidationError(form.errors)) return super().form_invalid(form) @transaction.atomic def form_valid(self, form): - form.save() + try: + form.save() + except ValidationError: + return self.form_invalid(form) return redirect(reverse("settings.teams")) From 88ddc7f6cd73399f314af92f997be07c5f431c4b Mon Sep 17 00:00:00 2001 From: Roffenlund Date: Mon, 22 Sep 2025 13:40:03 +0300 Subject: [PATCH 13/61] Add remove team member endpoint to tests Add remove team member endpoint tests. Update test_cyberstorm_DELETE_endpoint_schemas to create a team and add member to team. Update get_parameter_values. Refs. TS-2637 --- .../api/cyberstorm/tests/endpoint_data.py | 1 + .../api/cyberstorm/tests/test_endpoints.py | 9 +++++++-- django/thunderstore/api/cyberstorm/tests/utils.py | 11 +++++++++-- 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/django/thunderstore/api/cyberstorm/tests/endpoint_data.py b/django/thunderstore/api/cyberstorm/tests/endpoint_data.py index 8c04328a7..4b2511de0 100644 --- a/django/thunderstore/api/cyberstorm/tests/endpoint_data.py +++ b/django/thunderstore/api/cyberstorm/tests/endpoint_data.py @@ -48,6 +48,7 @@ }, "DELETE": [ "/api/cyberstorm/team/{team_name}/disband/", + "/api/cyberstorm/team/{team_name}/member/{username}/remove/", ], } diff --git a/django/thunderstore/api/cyberstorm/tests/test_endpoints.py b/django/thunderstore/api/cyberstorm/tests/test_endpoints.py index 674015a4b..1e5cf698e 100644 --- a/django/thunderstore/api/cyberstorm/tests/test_endpoints.py +++ b/django/thunderstore/api/cyberstorm/tests/test_endpoints.py @@ -18,6 +18,7 @@ validate_response_against_schema, ) from thunderstore.api.urls import cyberstorm_urls +from thunderstore.core.factories import UserFactory @pytest.mark.django_db @@ -60,14 +61,18 @@ def test_cyberstorm_DELETE_endpoint_schemas( service_account.owner = active_package_listing.package.owner service_account.save(update_fields=("owner",)) - param_values = get_parameter_values(active_package_listing) + team = user.teams.first().team + team_member = UserFactory.create() + team.add_member(team_member, role="member") + + param_values = get_parameter_values(active_package_listing, team_member.username) schema = get_schema(api_client) resolver = get_resolver(schema) url = fill_path_params(api_path, param_values) if "disband" in api_path: - user.teams.first().team.owned_packages.all().delete() + team.owned_packages.all().delete() # Cannot disband a team with packages response = api_client.delete(url, format="json") diff --git a/django/thunderstore/api/cyberstorm/tests/utils.py b/django/thunderstore/api/cyberstorm/tests/utils.py index 8ddbb9cb9..c6ccf48ff 100644 --- a/django/thunderstore/api/cyberstorm/tests/utils.py +++ b/django/thunderstore/api/cyberstorm/tests/utils.py @@ -10,10 +10,12 @@ from thunderstore.repository.models import PackageListing, TeamMemberRole -def get_parameter_values(package_listing: PackageListing) -> dict: +def get_parameter_values( + package_listing: PackageListing, username: Optional[str] = None +) -> dict: service_account = package_listing.package.owner.service_accounts.first() - return { + parameters = { "community_id": package_listing.community.identifier, "namespace_id": package_listing.package.owner.get_namespace().name, "package_name": package_listing.package.name, @@ -23,6 +25,11 @@ def get_parameter_values(package_listing: PackageListing) -> dict: "uuid": service_account.uuid if service_account else "", } + if username: + parameters["username"] = username + + return parameters + def setup_superuser_with_package(package_listing, package_category=None): user = UserFactory.create(is_superuser=True) From 5f83436968375d81d5c438b79398dba25af956f7 Mon Sep 17 00:00:00 2001 From: Roffenlund Date: Mon, 22 Sep 2025 16:36:26 +0300 Subject: [PATCH 14/61] Add query count test * Add new test to query count tests * Remove the existing query count tests in test file * Update util for asserting query count to check response status code * Add endpoint to endpoint_data Refs. TS-2639 --- .../api/cyberstorm/tests/endpoint_data.py | 1 + .../tests/test_package_version_list.py | 24 --------- .../api/cyberstorm/tests/test_query_count.py | 52 ++++++++++++++++--- .../api/cyberstorm/tests/utils.py | 7 +++ 4 files changed, 53 insertions(+), 31 deletions(-) diff --git a/django/thunderstore/api/cyberstorm/tests/endpoint_data.py b/django/thunderstore/api/cyberstorm/tests/endpoint_data.py index 8c04328a7..c98acb733 100644 --- a/django/thunderstore/api/cyberstorm/tests/endpoint_data.py +++ b/django/thunderstore/api/cyberstorm/tests/endpoint_data.py @@ -12,6 +12,7 @@ "/api/cyberstorm/package/{namespace_id}/{package_name}/latest/readme/", "/api/cyberstorm/package/{namespace_id}/{package_name}/v/{version_number}/changelog/", "/api/cyberstorm/package/{namespace_id}/{package_name}/v/{version_number}/readme/", + "/api/cyberstorm/package/{namespace_id}/{package_name}/v/{version_number}/dependencies/", "/api/cyberstorm/package/{namespace_id}/{package_name}/versions/", "/api/cyberstorm/team/{team_id}/", "/api/cyberstorm/team/{team_id}/member/", diff --git a/django/thunderstore/api/cyberstorm/tests/test_package_version_list.py b/django/thunderstore/api/cyberstorm/tests/test_package_version_list.py index bd399cb08..2ccbae4e2 100644 --- a/django/thunderstore/api/cyberstorm/tests/test_package_version_list.py +++ b/django/thunderstore/api/cyberstorm/tests/test_package_version_list.py @@ -96,30 +96,6 @@ def test_package_version_dependencies_list( assert response.json()["count"] == dependency_count -@pytest.mark.django_db -def test_package_version_dependencies_query_count(api_client: APIClient) -> None: - """Ensure that the number of database queries remains low regardless with 100 dependencies.""" - - dependency_count = 100 - query_count = 25 - - package = PackageFactory(name="TestPackage") - PackageVersionFactory(package=package) - - target_dependencies = [PackageVersionFactory() for _ in range(dependency_count)] - package.latest.dependencies.set([dep.id for dep in target_dependencies]) - - namespace = package.namespace.name - package_name = package.name - url = f"/api/cyberstorm/package/{namespace}/{package_name}/v/latest/dependencies/" - - with CaptureQueriesContext(connection) as ctx: - response = api_client.get(url) - - assert response.status_code == 200 - assert len(ctx.captured_queries) <= query_count - - @pytest.mark.django_db def test_package_version_dependencies_list_response(api_client: APIClient) -> None: """Test the response structure of the PackageVersionDependenciesListAPIView.""" diff --git a/django/thunderstore/api/cyberstorm/tests/test_query_count.py b/django/thunderstore/api/cyberstorm/tests/test_query_count.py index 9609d665d..86d68fe99 100644 --- a/django/thunderstore/api/cyberstorm/tests/test_query_count.py +++ b/django/thunderstore/api/cyberstorm/tests/test_query_count.py @@ -1,12 +1,14 @@ import factory import pytest +from rest_framework.test import APIClient from thunderstore.account.forms import CreateServiceAccountForm from thunderstore.api.cyberstorm.tests.endpoint_data import GET_TEST_CASES -from thunderstore.community.models import Community +from thunderstore.community.models import Community, PackageCategory from thunderstore.core.factories import UserFactory from thunderstore.repository.factories import PackageFactory, PackageVersionFactory from thunderstore.repository.models import ( + Package, PackageListing, Team, TeamMember, @@ -26,7 +28,10 @@ @pytest.mark.django_db @pytest.mark.parametrize("test_case", GET_TEST_CASES) def test_cyberstorm_api_GET_query_count( - test_case, api_client, active_package_listing, package_category + test_case: str, + api_client: APIClient, + active_package_listing: PackageListing, + package_category: PackageCategory, ): api_path = test_case["path"] user = setup_superuser_with_package(active_package_listing, package_category) @@ -43,7 +48,7 @@ def test_cyberstorm_api_GET_query_count( @pytest.mark.django_db -def test_cyberstorm_community_list_query_count(api_client): +def test_cyberstorm_community_list_query_count(api_client: APIClient): path = "/api/cyberstorm/community/" communities = [] @@ -64,7 +69,10 @@ def test_cyberstorm_community_list_query_count(api_client): @pytest.mark.django_db -def test_cyberstorm_package_versions_list_query_count(api_client, active_package): +def test_cyberstorm_package_versions_list_query_count( + api_client: APIClient, + active_package: Package, +): url = "/api/cyberstorm/package/{namespace_id}/{package_name}/versions/" PackageVersionFactory.create_batch( @@ -102,7 +110,9 @@ def test_cyberstorm_package_versions_list_query_count(api_client, active_package {"path": "/api/cyberstorm/listing/{community_id}/{namespace_id}/"}, ], ) -def test_cyberstorm_package_listing_list_query_count(test_case, api_client): +def test_cyberstorm_package_listing_list_query_count( + test_case: str, api_client: APIClient +): amount = 20 community = Community.objects.create( name="Test_Community", identifier="test_community" @@ -148,7 +158,7 @@ def test_cyberstorm_package_listing_list_query_count(test_case, api_client): @pytest.mark.django_db -def test_cyberstorm_team_member_list_query_count(api_client): +def test_cyberstorm_team_member_list_query_count(api_client: APIClient): url = "/api/cyberstorm/team/{team_id}/member/" super_user = UserFactory.create(is_superuser=True) team = Team.create(name="Test_Team") @@ -173,7 +183,7 @@ def test_cyberstorm_team_member_list_query_count(api_client): @pytest.mark.django_db -def test_cyberstorm_team_service_accounts_list_query_count(api_client): +def test_cyberstorm_team_service_accounts_list_query_count(api_client: APIClient): url = "/api/cyberstorm/team/{team_id}/service-account/" user = UserFactory.create(is_superuser=True) @@ -200,3 +210,31 @@ def test_cyberstorm_team_service_accounts_list_query_count(api_client): path=url, max_queries=MAX_QUERIES, ) + + +@pytest.mark.django_db +def test_package_version_dependencies_query_count(api_client: APIClient) -> None: + dependency_count = 20 # One page of dependencies + + package = PackageFactory(name="TestPackage") + PackageVersionFactory(package=package) + + target_dependencies = [PackageVersionFactory() for _ in range(dependency_count)] + package.latest.dependencies.set([dep.id for dep in target_dependencies]) + + url = "/api/cyberstorm/package/{namespace_id}/{package_name}/v/{version}/dependencies/" + + path_params = { + "namespace_id": package.namespace.name, + "package_name": package.name, + "version": "latest", + } + + path = fill_path_params(url, path_params) + + validate_max_queries( + client=api_client, + method="get", + path=path, + max_queries=25, # TODO: this could be optimized further + ) diff --git a/django/thunderstore/api/cyberstorm/tests/utils.py b/django/thunderstore/api/cyberstorm/tests/utils.py index 8ddbb9cb9..d71f5e267 100644 --- a/django/thunderstore/api/cyberstorm/tests/utils.py +++ b/django/thunderstore/api/cyberstorm/tests/utils.py @@ -215,6 +215,13 @@ def validate_max_queries( with CaptureQueriesContext(connection) as ctx: response = request_func(path, data=data or {}, **kwargs) + allowed_statuses = [200, 201, 202, 204] + if response.status_code not in allowed_statuses: + raise AssertionError( + f"{method} {path} returned status {response.status_code}, " + f"expected one of {allowed_statuses}." + ) + num_queries = len(ctx.captured_queries) if num_queries > max_queries: queries_str = "\n".join(q["sql"] for q in ctx.captured_queries) From 46fa16ebe747050fd3fa71e35570ea50f0a53d33 Mon Sep 17 00:00:00 2001 From: Roffenlund Date: Mon, 22 Sep 2025 16:44:46 +0300 Subject: [PATCH 15/61] Fix broken test Fix test which returned a 403 due to faulty setup. Refs. TS-2639 --- .../api/cyberstorm/tests/test_query_count.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/django/thunderstore/api/cyberstorm/tests/test_query_count.py b/django/thunderstore/api/cyberstorm/tests/test_query_count.py index 86d68fe99..49960e59c 100644 --- a/django/thunderstore/api/cyberstorm/tests/test_query_count.py +++ b/django/thunderstore/api/cyberstorm/tests/test_query_count.py @@ -162,14 +162,11 @@ def test_cyberstorm_team_member_list_query_count(api_client: APIClient): url = "/api/cyberstorm/team/{team_id}/member/" super_user = UserFactory.create(is_superuser=True) team = Team.create(name="Test_Team") + team.add_member(user=super_user, role=TeamMemberRole.owner) users = UserFactory.create_batch(20) - TeamMember.objects.bulk_create( - [ - TeamMember(team=team, user=member_user, role=TeamMemberRole.member) - for member_user in users - ] - ) + for user in users: + team.add_member(user=user, role=TeamMemberRole.member) api_client.force_authenticate(super_user) url = fill_path_params(url, {"team_id": team.name}) From b893ad5cbbecb16fd2dde8f24ff0402a37b6dfe7 Mon Sep 17 00:00:00 2001 From: Roffenlund Date: Tue, 2 Sep 2025 11:45:33 +0300 Subject: [PATCH 16/61] Implement listing status serializer Implement serializer to be user for serializing package listing status responses. Refs. TS-2598 --- django/thunderstore/api/cyberstorm/serializers/__init__.py | 2 ++ .../api/cyberstorm/serializers/package_listing.py | 7 +++++++ 2 files changed, 9 insertions(+) diff --git a/django/thunderstore/api/cyberstorm/serializers/__init__.py b/django/thunderstore/api/cyberstorm/serializers/__init__.py index e59a52700..29151a698 100644 --- a/django/thunderstore/api/cyberstorm/serializers/__init__.py +++ b/django/thunderstore/api/cyberstorm/serializers/__init__.py @@ -8,6 +8,7 @@ CyberstormPackagePreviewSerializer, PackagePermissionsSerializer, ) +from .package_listing import PackageListingStatusResponseSerializer from .team import ( CyberstormCreateTeamSerializer, CyberstormServiceAccountSerializer, @@ -32,4 +33,5 @@ "PackagePermissionsSerializer", "CyberstormTeamUpdateSerializer", "CyberstormPackageDependencySerializer", + "PackageListingStatusResponseSerializer", ] diff --git a/django/thunderstore/api/cyberstorm/serializers/package_listing.py b/django/thunderstore/api/cyberstorm/serializers/package_listing.py index af2d23dba..a4aa89aba 100644 --- a/django/thunderstore/api/cyberstorm/serializers/package_listing.py +++ b/django/thunderstore/api/cyberstorm/serializers/package_listing.py @@ -35,3 +35,10 @@ class PackageListingApproveSerializer(serializers.Serializer): internal_notes = serializers.CharField( allow_blank=True, allow_null=True, required=False ) + + +class PackageListingStatusResponseSerializer(serializers.Serializer): + review_status = serializers.CharField(required=False, allow_null=True) + rejection_reason = serializers.CharField(required=False, allow_null=True) + internal_notes = serializers.CharField(required=False, allow_null=True) + listing_admin_url = serializers.CharField(required=False, allow_null=True) From a85bf5573b9a8b0f5a59e072c99bc0700dbe4ce0 Mon Sep 17 00:00:00 2001 From: Roffenlund Date: Tue, 2 Sep 2025 11:46:48 +0300 Subject: [PATCH 17/61] Implement view for getting listing status Implement a view which fetches internal_notes, rejection_reason, review_status, listing_admin_url for package listings. Return the correct data depending on what permissions the user has. These permissions mirror the legacy view's permissions. Implement unit tests and URL for the endpoint. Refs. TS-2598 --- .../cyberstorm/tests/test_package_listing.py | 116 +++++++++++++++++- .../api/cyberstorm/views/__init__.py | 3 +- .../api/cyberstorm/views/package_listing.py | 55 ++++++++- django/thunderstore/api/urls.py | 6 + 4 files changed, 174 insertions(+), 6 deletions(-) diff --git a/django/thunderstore/api/cyberstorm/tests/test_package_listing.py b/django/thunderstore/api/cyberstorm/tests/test_package_listing.py index 8e4629f25..5f6d64b5b 100644 --- a/django/thunderstore/api/cyberstorm/tests/test_package_listing.py +++ b/django/thunderstore/api/cyberstorm/tests/test_package_listing.py @@ -7,6 +7,7 @@ from django.test.utils import CaptureQueriesContext from rest_framework.test import APIClient +from conftest import TestUserTypes from thunderstore.api.cyberstorm.views.package_listing import ( DependencySerializer, get_custom_package_listing, @@ -139,7 +140,9 @@ def test_get_custom_package_listing__augments_listing_with_dependency_count() -> @pytest.mark.django_db -def test_get_custom_package_listing__augments_listing_with_dependencies_from_same_community() -> None: +def test_get_custom_package_listing__augments_listing_with_dependencies_from_same_community() -> ( + None +): dependant = PackageListingFactory() dependency1 = PackageListingFactory(community=dependant.community) dependency2 = PackageListingFactory() @@ -165,7 +168,9 @@ def test_get_custom_package_listing__augments_listing_with_dependencies_from_sam @pytest.mark.django_db -def test_get_custom_package_listing__when_many_dependencies__returns_only_four() -> None: +def test_get_custom_package_listing__when_many_dependencies__returns_only_four() -> ( + None +): listing = PackageListingFactory() dependency_count = 6 dependencies = PackageListingFactory.create_batch( @@ -295,7 +300,9 @@ def test_dependency_serializer__reads_is_active_from_correct_field( @pytest.mark.django_db -def test_dependency_serializer__when_dependency_is_not_active__censors_icon_and_description() -> None: +def test_dependency_serializer__when_dependency_is_not_active__censors_icon_and_description() -> ( + None +): dependency = PackageVersionFactory() dependency.community_identifier = "greendale" dependency.version_is_unavailable = False @@ -420,3 +427,106 @@ def test_package_listing_query_count( assert response.status_code == 200 assert len(ctx.captured_queries) < 20 + + +@pytest.mark.django_db +@pytest.mark.parametrize("user_type", TestUserTypes.options()) +def test_get_package_listing_status( + active_package_listing, api_client, user_type +) -> None: + base_url = "/api/cyberstorm/listing" + community_id = active_package_listing.community.identifier + namespace_id = active_package_listing.package.namespace.name + package_name = active_package_listing.package.name + url = f"{base_url}/{community_id}/{namespace_id}/{package_name}/status/" + + user = TestUserTypes.get_user_by_type(user_type) + + is_fake_user = user_type in TestUserTypes.fake_users() + is_unauthenticated = user_type == TestUserTypes.unauthenticated + + if not is_fake_user and not is_unauthenticated: + api_client.force_authenticate(user=user) + + response = api_client.get(url) + + expected_status_code = { + TestUserTypes.no_user: 401, + TestUserTypes.unauthenticated: 401, + TestUserTypes.regular_user: 403, + TestUserTypes.deactivated_user: 403, + TestUserTypes.service_account: 403, + TestUserTypes.site_admin: 200, + TestUserTypes.superuser: 200, + } + + assert response.status_code == expected_status_code[user_type] + + +@pytest.mark.django_db +@pytest.mark.parametrize( + "can_manage_return_val,can_moderate_return_val,status_code", + [ + (True, True, 200), + (True, False, 200), + (False, True, 200), + (False, False, 403), + ], +) +@patch( + "thunderstore.repository.views.package.detail.PermissionsChecker.can_moderate", + new_callable=PropertyMock, +) +@patch( + "thunderstore.repository.views.package.detail.PermissionsChecker.can_manage", + new_callable=PropertyMock, +) +def test_get_package_listing_status_permissions( + mock_can_manage, + mock_can_moderate, + can_manage_return_val, + can_moderate_return_val, + status_code, + api_client, + active_package_listing, +): + mock_can_manage.return_value = can_manage_return_val + mock_can_moderate.return_value = can_moderate_return_val + + base_url = "/api/cyberstorm/listing" + community_id = active_package_listing.community.identifier + namespace_id = active_package_listing.package.namespace.name + package_name = active_package_listing.package.name + url = f"{base_url}/{community_id}/{namespace_id}/{package_name}/status/" + + active_package_listing.rejection_reason = "Inappropriate content" + active_package_listing.notes = "This package contains inappropriate content." + active_package_listing.review_status = "rejected" + active_package_listing.save() + + user = TestUserTypes.get_user_by_type(TestUserTypes.superuser) + api_client.force_authenticate(user=user) + response = api_client.get(url) + + assert response.status_code == status_code + data = response.json() + + expected = { + "review_status": None, + "rejection_reason": None, + "internal_notes": None, + "listing_admin_url": None, + } + + if can_manage_return_val: + expected["review_status"] = active_package_listing.review_status + expected["rejection_reason"] = active_package_listing.rejection_reason + expected["listing_admin_url"] = active_package_listing.get_admin_url() + + if can_moderate_return_val: + expected["internal_notes"] = active_package_listing.notes + + if not can_manage_return_val and not can_moderate_return_val: + expected = {"detail": "You do not have permission to view review information."} + + assert data == expected diff --git a/django/thunderstore/api/cyberstorm/views/__init__.py b/django/thunderstore/api/cyberstorm/views/__init__.py index 3e93f0a75..dfbacb257 100644 --- a/django/thunderstore/api/cyberstorm/views/__init__.py +++ b/django/thunderstore/api/cyberstorm/views/__init__.py @@ -3,7 +3,7 @@ from .community_list import CommunityListAPIView from .markdown import PackageVersionChangelogAPIView, PackageVersionReadmeAPIView from .package_deprecate import DeprecatePackageAPIView -from .package_listing import PackageListingAPIView +from .package_listing import PackageListingAPIView, PackageListingStatusAPIView from .package_listing_actions import ( ApprovePackageListingAPIView, RejectPackageListingAPIView, @@ -38,6 +38,7 @@ "DeprecatePackageAPIView", "DisbandTeamAPIView", "PackageListingAPIView", + "PackageListingStatusAPIView", "PackageListingByCommunityListAPIView", "PackageListingByDependencyListAPIView", "PackageListingByNamespaceListAPIView", diff --git a/django/thunderstore/api/cyberstorm/views/package_listing.py b/django/thunderstore/api/cyberstorm/views/package_listing.py index 6ec0eca67..80a5d7efa 100644 --- a/django/thunderstore/api/cyberstorm/views/package_listing.py +++ b/django/thunderstore/api/cyberstorm/views/package_listing.py @@ -12,18 +12,27 @@ Sum, Value, ) -from rest_framework import serializers +from drf_yasg.utils import swagger_auto_schema +from rest_framework import serializers, status +from rest_framework.exceptions import PermissionDenied from rest_framework.generics import RetrieveAPIView, get_object_or_404 +from rest_framework.permissions import IsAuthenticated +from rest_framework.response import Response +from rest_framework.views import APIView from thunderstore.api.cyberstorm.serializers import ( CyberstormPackageCategorySerializer, CyberstormTeamMemberSerializer, + PackageListingStatusResponseSerializer, +) +from thunderstore.api.cyberstorm.views.package_listing_actions import ( + get_package_listing, ) from thunderstore.api.utils import CyberstormAutoSchemaMixin -from thunderstore.community.models.community import Community from thunderstore.community.models.package_listing import PackageListing from thunderstore.repository.models.package import get_package_dependants from thunderstore.repository.models.package_version import PackageVersion +from thunderstore.repository.views.package.detail import PermissionsChecker class DependencySerializer(serializers.Serializer): @@ -218,3 +227,45 @@ def get_custom_package_listing( listing.dependant_count = get_package_dependants(listing.package.pk).count() return listing + + +class PackageListingStatusAPIView(APIView): + permission_classes = [IsAuthenticated] + serializer_class = PackageListingStatusResponseSerializer + + @swagger_auto_schema( + operation_id="cyberstorm.package_listing.status", + responses={200: serializer_class}, + tags=["cyberstorm"], + ) + def get( + self, request, namespace_id: str, package_name: str, community_id: str + ) -> Response: + package_listing = get_package_listing( + namespace_id=namespace_id, + package_name=package_name, + community_id=community_id, + ) + checker = PermissionsChecker(package_listing, request.user) + + if not (checker.can_manage or checker.can_moderate): + error_msg = "You do not have permission to view review information." + raise PermissionDenied(error_msg) + + response_data = { + "review_status": None, + "rejection_reason": None, + "internal_notes": None, + "listing_admin_url": None, + } + + if checker.can_manage: + response_data["review_status"] = package_listing.review_status + response_data["rejection_reason"] = package_listing.rejection_reason + response_data["listing_admin_url"] = package_listing.get_admin_url() + + if checker.can_moderate: + response_data["internal_notes"] = package_listing.notes + + serializer = self.serializer_class(response_data) + return Response(serializer.data, status=status.HTTP_200_OK) diff --git a/django/thunderstore/api/urls.py b/django/thunderstore/api/urls.py index daa351d57..8289190d6 100644 --- a/django/thunderstore/api/urls.py +++ b/django/thunderstore/api/urls.py @@ -11,6 +11,7 @@ PackageListingByCommunityListAPIView, PackageListingByDependencyListAPIView, PackageListingByNamespaceListAPIView, + PackageListingStatusAPIView, PackagePermissionsAPIView, PackageVersionChangelogAPIView, PackageVersionDependenciesListAPIView, @@ -59,6 +60,11 @@ PackageListingAPIView.as_view(), name="cyberstorm.listing", ), + path( + "listing////status/", + PackageListingStatusAPIView.as_view(), + name="cyberstorm.listing.status", + ), path( "listing////dependants/", PackageListingByDependencyListAPIView.as_view(), From 87ada425fea110165e96c30b2db0c28b69da6bcd Mon Sep 17 00:00:00 2001 From: Roffenlund Date: Fri, 5 Sep 2025 13:59:09 +0300 Subject: [PATCH 18/61] Add permission check for listing_admin_url Add a separate check for who can view the listing_admin_url. Refactor tests as the previous test would have gotten too big by adding one more permission. Refs. TS-2598 --- .../cyberstorm/tests/test_package_listing.py | 119 +++++++++++------- .../api/cyberstorm/views/package_listing.py | 2 + 2 files changed, 74 insertions(+), 47 deletions(-) diff --git a/django/thunderstore/api/cyberstorm/tests/test_package_listing.py b/django/thunderstore/api/cyberstorm/tests/test_package_listing.py index 5f6d64b5b..a738d4d0a 100644 --- a/django/thunderstore/api/cyberstorm/tests/test_package_listing.py +++ b/django/thunderstore/api/cyberstorm/tests/test_package_listing.py @@ -26,6 +26,16 @@ ) +def get_listing_url(package_listing) -> str: + base_url = "/api/cyberstorm/listing" + + community_id = package_listing.community.identifier + namespace_id = package_listing.package.namespace.name + package_name = package_listing.package.name + + return f"{base_url}/{community_id}/{namespace_id}/{package_name}/status/" + + @pytest.mark.django_db def test_get_custom_package_listing__returns_objects_matching_args() -> None: expected = PackageListingFactory() @@ -464,69 +474,84 @@ def test_get_package_listing_status( @pytest.mark.django_db -@pytest.mark.parametrize( - "can_manage_return_val,can_moderate_return_val,status_code", - [ - (True, True, 200), - (True, False, 200), - (False, True, 200), - (False, False, 403), - ], -) -@patch( - "thunderstore.repository.views.package.detail.PermissionsChecker.can_moderate", - new_callable=PropertyMock, -) +@pytest.mark.parametrize("return_val", [True, False]) @patch( "thunderstore.repository.views.package.detail.PermissionsChecker.can_manage", new_callable=PropertyMock, ) -def test_get_package_listing_status_permissions( - mock_can_manage, - mock_can_moderate, - can_manage_return_val, - can_moderate_return_val, - status_code, - api_client, - active_package_listing, +def test_package_listing_status_can_manage_permission( + mock_can_manage, return_val, api_client, active_package_listing ): - mock_can_manage.return_value = can_manage_return_val - mock_can_moderate.return_value = can_moderate_return_val - - base_url = "/api/cyberstorm/listing" - community_id = active_package_listing.community.identifier - namespace_id = active_package_listing.package.namespace.name - package_name = active_package_listing.package.name - url = f"{base_url}/{community_id}/{namespace_id}/{package_name}/status/" - active_package_listing.rejection_reason = "Inappropriate content" - active_package_listing.notes = "This package contains inappropriate content." active_package_listing.review_status = "rejected" active_package_listing.save() + mock_can_manage.return_value = return_val + user = TestUserTypes.get_user_by_type(TestUserTypes.superuser) api_client.force_authenticate(user=user) + + url = get_listing_url(active_package_listing) response = api_client.get(url) - assert response.status_code == status_code data = response.json() - expected = { - "review_status": None, - "rejection_reason": None, - "internal_notes": None, - "listing_admin_url": None, - } + if return_val: + assert data["review_status"] == "rejected" + assert data["rejection_reason"] == "Inappropriate content" + else: + assert data["review_status"] is None + assert data["rejection_reason"] is None - if can_manage_return_val: - expected["review_status"] = active_package_listing.review_status - expected["rejection_reason"] = active_package_listing.rejection_reason - expected["listing_admin_url"] = active_package_listing.get_admin_url() - if can_moderate_return_val: - expected["internal_notes"] = active_package_listing.notes +@pytest.mark.django_db +@pytest.mark.parametrize("return_val", [True, False]) +@patch( + "thunderstore.repository.views.package.detail.PermissionsChecker.can_view_listing_admin_page", + new_callable=PropertyMock, +) +def test_package_listing_status_can_view_listing_admin_page_permission( + mock_can_view_listing_admin_page, return_val, api_client, active_package_listing +): + mock_can_view_listing_admin_page.return_value = return_val + + user = TestUserTypes.get_user_by_type(TestUserTypes.superuser) + api_client.force_authenticate(user=user) + + url = get_listing_url(active_package_listing) + response = api_client.get(url) + + data = response.json() + + if return_val: + assert data["listing_admin_url"] == active_package_listing.get_admin_url() + else: + assert data["listing_admin_url"] is None - if not can_manage_return_val and not can_moderate_return_val: - expected = {"detail": "You do not have permission to view review information."} - assert data == expected +@pytest.mark.django_db +@pytest.mark.parametrize("return_val", [True, False]) +@patch( + "thunderstore.repository.views.package.detail.PermissionsChecker.can_moderate", + new_callable=PropertyMock, +) +def test_package_listing_status_can_moderate_permission( + mock_can_moderate, return_val, api_client, active_package_listing +): + mock_can_moderate.return_value = return_val + + active_package_listing.notes = "This package contains inappropriate content." + active_package_listing.save() + + user = TestUserTypes.get_user_by_type(TestUserTypes.superuser) + api_client.force_authenticate(user=user) + + url = get_listing_url(active_package_listing) + response = api_client.get(url) + + data = response.json() + + if return_val: + assert data["internal_notes"] == "This package contains inappropriate content." + else: + assert data["internal_notes"] is None diff --git a/django/thunderstore/api/cyberstorm/views/package_listing.py b/django/thunderstore/api/cyberstorm/views/package_listing.py index 80a5d7efa..2e5fff828 100644 --- a/django/thunderstore/api/cyberstorm/views/package_listing.py +++ b/django/thunderstore/api/cyberstorm/views/package_listing.py @@ -262,6 +262,8 @@ def get( if checker.can_manage: response_data["review_status"] = package_listing.review_status response_data["rejection_reason"] = package_listing.rejection_reason + + if checker.can_view_listing_admin_page: response_data["listing_admin_url"] = package_listing.get_admin_url() if checker.can_moderate: From a0fb68768ab89eb26ee461202dac28f44013d805 Mon Sep 17 00:00:00 2001 From: Roffenlund Date: Tue, 23 Sep 2025 12:00:25 +0300 Subject: [PATCH 19/61] Add endpoint to endpoint tests Refs. TS-2598 --- django/thunderstore/api/cyberstorm/tests/endpoint_data.py | 1 + 1 file changed, 1 insertion(+) diff --git a/django/thunderstore/api/cyberstorm/tests/endpoint_data.py b/django/thunderstore/api/cyberstorm/tests/endpoint_data.py index 6613aeb7f..b0d5cbce6 100644 --- a/django/thunderstore/api/cyberstorm/tests/endpoint_data.py +++ b/django/thunderstore/api/cyberstorm/tests/endpoint_data.py @@ -6,6 +6,7 @@ "/api/cyberstorm/listing/{community_id}/", "/api/cyberstorm/listing/{community_id}/{namespace_id}/", "/api/cyberstorm/listing/{community_id}/{namespace_id}/{package_name}/", + "/api/cyberstorm/listing/{community_id}/{namespace_id}/{package_name}/status/", "/api/cyberstorm/listing/{community_id}/{namespace_id}/{package_name}/dependants/", "/api/cyberstorm/package/{community_id}/{namespace_id}/{package_name}/permissions/", "/api/cyberstorm/package/{namespace_id}/{package_name}/latest/changelog/", From 1ac0481fcde7422f1aebab7844e6d68d361b0e04 Mon Sep 17 00:00:00 2001 From: Roffenlund Date: Wed, 24 Sep 2025 18:35:17 +0300 Subject: [PATCH 20/61] Implement team services Implement services for the following: * Create service account * Delete service account Implement tests for the services. Refs. TS-2320 --- .../api/cyberstorm/services/team.py | 23 ++++++ .../tests/services/test_team_services.py | 82 +++++++++++++++++++ 2 files changed, 105 insertions(+) diff --git a/django/thunderstore/api/cyberstorm/services/team.py b/django/thunderstore/api/cyberstorm/services/team.py index 09043ff39..02e79577b 100644 --- a/django/thunderstore/api/cyberstorm/services/team.py +++ b/django/thunderstore/api/cyberstorm/services/team.py @@ -2,6 +2,7 @@ from django.db import transaction from django.shortcuts import get_object_or_404 +from thunderstore.account.models import ServiceAccount from thunderstore.core.exceptions import PermissionValidationError from thunderstore.core.types import UserType from thunderstore.repository.models import Namespace, Team @@ -50,3 +51,25 @@ def remove_team_member(agent: UserType, member: TeamMember) -> None: member.team.ensure_user_can_manage_members(agent) member.team.ensure_member_can_be_removed(member) member.delete() + + +@transaction.atomic +def create_service_account(agent: UserType, team: Team, nickname: str): + team.ensure_user_can_access(agent) + team.ensure_can_create_service_account(agent) + + service_account, token = ServiceAccount.create( + owner=team, + nickname=nickname, + creator=agent, + ) + + return service_account, token + + +@transaction.atomic +def delete_service_account(agent: UserType, service_account: ServiceAccount): + team = service_account.owner + team.ensure_user_can_access(agent) + team.ensure_can_delete_service_account(agent) + return service_account.delete() diff --git a/django/thunderstore/api/cyberstorm/tests/services/test_team_services.py b/django/thunderstore/api/cyberstorm/tests/services/test_team_services.py index 2a8b7a05f..526bfc1c0 100644 --- a/django/thunderstore/api/cyberstorm/tests/services/test_team_services.py +++ b/django/thunderstore/api/cyberstorm/tests/services/test_team_services.py @@ -3,6 +3,7 @@ from django.http import Http404 from conftest import TestUserTypes +from thunderstore.account.factories import UserFactory from thunderstore.api.cyberstorm.services import team as team_services from thunderstore.core.exceptions import PermissionValidationError from thunderstore.repository.models import Namespace, Team, TeamMemberRole @@ -168,3 +169,84 @@ def test_remove_team_member_from_another_team(team_owner, user): team_services.remove_team_member( agent=team_owner.user, member=another_team_member ) + + +@pytest.mark.django_db +@pytest.mark.parametrize("user_type", TestUserTypes.options()) +def test_create_service_account_user_types(user_type: str, team): + user = TestUserTypes.get_user_by_type(user_type) + + user_type_result = { + TestUserTypes.no_user: False, + TestUserTypes.unauthenticated: False, + TestUserTypes.regular_user: True, + TestUserTypes.deactivated_user: False, + TestUserTypes.service_account: False, + TestUserTypes.site_admin: True, + TestUserTypes.superuser: True, + } + + should_raise_error = user_type_result[user_type] is False + + if user_type not in [TestUserTypes.no_user, TestUserTypes.unauthenticated]: + team.add_member(user=user, role=TeamMemberRole.owner) + + if should_raise_error: + with pytest.raises(PermissionValidationError): + team_services.create_service_account(user, team, "TestServiceAccount") + else: + service_account, token = team_services.create_service_account( + user, team, "TestServiceAccount" + ) + assert service_account.owner == team + assert service_account.nickname == "TestServiceAccount" + assert token is not None + assert token.startswith("tss_") + + +@pytest.mark.django_db +def test_create_service_account_not_owner(team_member): + nickname = "TestServiceAccount" + error_msg = "Must be an owner to create a service account" + with pytest.raises(PermissionValidationError, match=error_msg): + team_services.create_service_account( + team_member.user, team_member.team, nickname + ) + + +@pytest.mark.django_db +@pytest.mark.parametrize("user_type", TestUserTypes.options()) +def test_delete_service_account_user_types(user_type: str, service_account): + user = TestUserTypes.get_user_by_type(user_type) + + user_type_result = { + TestUserTypes.no_user: False, + TestUserTypes.unauthenticated: False, + TestUserTypes.regular_user: True, + TestUserTypes.deactivated_user: False, + TestUserTypes.service_account: False, + TestUserTypes.site_admin: True, + TestUserTypes.superuser: True, + } + + should_raise_error = user_type_result[user_type] is False + team = service_account.owner + + if user_type not in [TestUserTypes.no_user, TestUserTypes.unauthenticated]: + team.add_member(user=user, role=TeamMemberRole.owner) + + if should_raise_error: + with pytest.raises(PermissionValidationError): + team_services.delete_service_account(user, service_account) + else: + team_services.delete_service_account(user, service_account) + assert service_account.pk is None + + +@pytest.mark.django_db +def test_delete_service_account_not_owner(service_account): + user = UserFactory() + service_account.owner.add_member(user=user, role=TeamMemberRole.member) + error_msg = "Must be an owner to delete a service account" + with pytest.raises(PermissionValidationError, match=error_msg): + team_services.delete_service_account(user, service_account) From 6821d019830c353927a8b4e303c404dbfea657f1 Mon Sep 17 00:00:00 2001 From: Roffenlund Date: Wed, 24 Sep 2025 18:36:39 +0300 Subject: [PATCH 21/61] Implement serializer for create service account Refs. TS-2320 --- django/thunderstore/api/cyberstorm/serializers/__init__.py | 2 ++ django/thunderstore/api/cyberstorm/serializers/team.py | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/django/thunderstore/api/cyberstorm/serializers/__init__.py b/django/thunderstore/api/cyberstorm/serializers/__init__.py index 29151a698..83f560393 100644 --- a/django/thunderstore/api/cyberstorm/serializers/__init__.py +++ b/django/thunderstore/api/cyberstorm/serializers/__init__.py @@ -10,6 +10,7 @@ ) from .package_listing import PackageListingStatusResponseSerializer from .team import ( + CyberstormCreateServiceAccountSerializer, CyberstormCreateTeamSerializer, CyberstormServiceAccountSerializer, CyberstormTeamAddMemberRequestSerializer, @@ -23,6 +24,7 @@ "CyberstormTeamAddMemberRequestSerializer", "CyberstormTeamAddMemberResponseSerializer", "CyberstormCreateTeamSerializer", + "CyberstormCreateServiceAccountSerializer", "CyberstormCommunitySerializer", "CyberstormPackageCategorySerializer", "CyberstormPackageListingSectionSerializer", diff --git a/django/thunderstore/api/cyberstorm/serializers/team.py b/django/thunderstore/api/cyberstorm/serializers/team.py index 4658acf56..f65d4215b 100644 --- a/django/thunderstore/api/cyberstorm/serializers/team.py +++ b/django/thunderstore/api/cyberstorm/serializers/team.py @@ -60,3 +60,9 @@ class CyberstormTeamUpdateSerializer(serializers.Serializer): donation_link = serializers.CharField( max_length=1024, validators=[URLValidator(["https"])] ) + + +class CyberstormCreateServiceAccountSerializer(serializers.Serializer): + nickname = serializers.CharField(max_length=32) + team_name = serializers.CharField(read_only=True) + api_token = serializers.CharField(read_only=True) From 7c1270f0600d21284b9ed342b1da9b67b7a0d392 Mon Sep 17 00:00:00 2001 From: Roffenlund Date: Wed, 24 Sep 2025 18:37:43 +0300 Subject: [PATCH 22/61] Implement create & delete service account views Refs. TS-2320 --- .../api/cyberstorm/views/__init__.py | 4 ++ .../thunderstore/api/cyberstorm/views/team.py | 47 +++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/django/thunderstore/api/cyberstorm/views/__init__.py b/django/thunderstore/api/cyberstorm/views/__init__.py index dfbacb257..f2ea3e85e 100644 --- a/django/thunderstore/api/cyberstorm/views/__init__.py +++ b/django/thunderstore/api/cyberstorm/views/__init__.py @@ -21,6 +21,8 @@ PackageVersionListAPIView, ) from .team import ( + CreateServiceAccountAPIView, + DeleteServiceAccountAPIView, DisbandTeamAPIView, TeamAPIView, TeamCreateAPIView, @@ -35,6 +37,8 @@ "CommunityAPIView", "CommunityFiltersAPIView", "CommunityListAPIView", + "CreateServiceAccountAPIView", + "DeleteServiceAccountAPIView", "DeprecatePackageAPIView", "DisbandTeamAPIView", "PackageListingAPIView", diff --git a/django/thunderstore/api/cyberstorm/views/team.py b/django/thunderstore/api/cyberstorm/views/team.py index 0c6bcea1d..c38d1bf57 100644 --- a/django/thunderstore/api/cyberstorm/views/team.py +++ b/django/thunderstore/api/cyberstorm/views/team.py @@ -9,6 +9,7 @@ from thunderstore.account.models.service_account import ServiceAccount from thunderstore.api.cyberstorm.serializers import ( + CyberstormCreateServiceAccountSerializer, CyberstormCreateTeamSerializer, CyberstormServiceAccountSerializer, CyberstormTeamAddMemberRequestSerializer, @@ -18,7 +19,9 @@ CyberstormTeamUpdateSerializer, ) from thunderstore.api.cyberstorm.services.team import ( + create_service_account, create_team, + delete_service_account, disband_team, remove_team_member, update_team, @@ -166,6 +169,50 @@ def delete(self, request, *args, **kwargs): return Response(status=status.HTTP_204_NO_CONTENT) +class CreateServiceAccountAPIView(APIView): + permission_classes = [IsAuthenticated] + + @conditional_swagger_auto_schema( + request_body=CyberstormCreateServiceAccountSerializer, + operation_id="cyberstorm.team.service-account.create", + tags=["cyberstorm"], + responses={status.HTTP_201_CREATED: CyberstormCreateServiceAccountSerializer}, + ) + def post(self, request, *args, **kwargs): + serializer = CyberstormCreateServiceAccountSerializer(data=request.data) + serializer.is_valid(raise_exception=True) + + team = get_object_or_404(Team, name=kwargs["team_name"]) + + service_account, token = create_service_account( + agent=request.user, + team=team, + nickname=serializer.validated_data["nickname"], + ) + + response_data = { + "nickname": service_account.nickname, + "team_name": service_account.owner.name, + "api_token": token, + } + + return Response(response_data, status=status.HTTP_201_CREATED) + + +class DeleteServiceAccountAPIView(APIView): + permission_classes = [IsAuthenticated] + + @conditional_swagger_auto_schema( + operation_id="cyberstorm.team.service-account.delete", + tags=["cyberstorm"], + responses={status.HTTP_204_NO_CONTENT: ""}, + ) + def delete(self, request, *args, **kwargs): + service_account = get_object_or_404(ServiceAccount, uuid=kwargs["uuid"]) + delete_service_account(agent=request.user, service_account=service_account) + return Response(status=status.HTTP_204_NO_CONTENT) + + class UpdateTeamAPIView(APIView): permission_classes = [IsAuthenticated] serializer_class = CyberstormTeamUpdateSerializer From e665e3891815797013f4a3353b0614eb165e2de4 Mon Sep 17 00:00:00 2001 From: Roffenlund Date: Wed, 24 Sep 2025 18:38:30 +0300 Subject: [PATCH 23/61] Implement create/delete service account endpoints Refs. TS-2320 --- .../cyberstorm/tests/test_service_account.py | 205 ++++++++++++++++++ django/thunderstore/api/urls.py | 12 + 2 files changed, 217 insertions(+) create mode 100644 django/thunderstore/api/cyberstorm/tests/test_service_account.py diff --git a/django/thunderstore/api/cyberstorm/tests/test_service_account.py b/django/thunderstore/api/cyberstorm/tests/test_service_account.py new file mode 100644 index 000000000..05ca7cd60 --- /dev/null +++ b/django/thunderstore/api/cyberstorm/tests/test_service_account.py @@ -0,0 +1,205 @@ +import json + +import pytest +from django.contrib.auth import get_user_model +from rest_framework.test import APIClient + +from thunderstore.account.models.service_account import ServiceAccount +from thunderstore.repository.models.team import Team, TeamMember + +User = get_user_model() + + +def get_create_service_account_url(team_name: str) -> str: + return f"/api/cyberstorm/team/{team_name}/service-account/create/" + + +def get_delete_service_account_url(team_name: str, uuid: str) -> str: + return f"/api/cyberstorm/team/{team_name}/service-account/delete/{uuid}/" + + +@pytest.mark.django_db +def test_create_service_account_success(api_client: APIClient, team_owner: TeamMember): + api_client.force_authenticate(team_owner.user) + + url = get_create_service_account_url(team_owner.team.name) + data = json.dumps({"nickname": "CoolestTeamServiceAccountName"}) + + response = api_client.post(url, data, content_type="application/json") + + expected_response = { + "nickname": "CoolestTeamServiceAccountName", + "team_name": team_owner.team.name, + "api_token": "tss_", + } + + service_account_count = ServiceAccount.objects.filter( + owner__name=team_owner.team.name, + user__first_name="CoolestTeamServiceAccountName", + ).count() + + assert response.status_code == 201 + assert response.json()["nickname"] == expected_response["nickname"] + assert response.json()["team_name"] == expected_response["team_name"] + assert response.json()["api_token"][:4] == expected_response["api_token"] + assert service_account_count == 1 + + +@pytest.mark.django_db +def test_create_service_account_not_authenticated( + api_client: APIClient, team_owner: TeamMember +): + url = get_create_service_account_url(team_owner.team.name) + data = json.dumps({"nickname": "CoolestTeamServiceAccountName"}) + + response = api_client.post(url, data, content_type="application/json") + expected_response = {"detail": "Authentication credentials were not provided."} + + assert response.status_code == 401 + assert response.json() == expected_response + + +@pytest.mark.django_db +def test_create_service_account_fails_because_nickname_too_long( + api_client: APIClient, + team_owner: TeamMember, +): + api_client.force_authenticate(team_owner.user) + url = get_create_service_account_url(team_owner.team.name) + data = json.dumps({"nickname": "LongestCoolestTeamServiceAccountNameEver"}) + + response = api_client.post(url, data, content_type="application/json") + + expected_response = { + "nickname": ["Ensure this field has no more than 32 characters."] + } + + service_account_count = ServiceAccount.objects.filter( + owner__name=team_owner.team.name, + user__first_name="LongestCoolestTeamServiceAccountNameEver", + ).count() + + assert response.status_code == 400 + assert response.json() == expected_response + assert service_account_count == 0 + + +@pytest.mark.django_db +def test_create_service_account_fail_because_user_is_not_team_member( + api_client: APIClient, + team: Team, +): + non_team_user = User.objects.create() + api_client.force_authenticate(non_team_user) + + url = get_create_service_account_url(team.name) + data = json.dumps({"nickname": "CoolestTeamServiceAccountName"}) + + response = api_client.post(url, data, content_type="application/json") + account_count = ServiceAccount.objects.filter( + owner__name=team.name, user__first_name="CoolestTeamServiceAccountName" + ).count() + + expected_response = {"non_field_errors": ["Must be a member to access team"]} + + assert response.status_code == 403 + assert account_count == 0 + assert response.json() == expected_response + + +@pytest.mark.django_db +def test_create_service_account_fail_because_user_is_not_team_owner( + api_client: APIClient, + team: Team, + team_member: TeamMember, +): + api_client.force_authenticate(team_member.user) + url = get_create_service_account_url(team.name) + data = json.dumps({"nickname": "CoolestTeamServiceAccountName"}) + + response = api_client.post(url, data, content_type="application/json") + account_count = ServiceAccount.objects.filter( + owner__name=team.name, user__first_name="CoolestTeamServiceAccountName" + ).count() + + expected_response = { + "non_field_errors": ["Must be an owner to create a service account"] + } + + assert response.status_code == 403 + assert account_count == 0 + assert response.json() == expected_response + + +@pytest.mark.django_db +def test_delete_service_account_success( + api_client: APIClient, + team_owner: TeamMember, + service_account: ServiceAccount, +): + assert ServiceAccount.objects.filter(uuid=service_account.uuid).count() == 1 + + api_client.force_authenticate(team_owner.user) + url = get_delete_service_account_url(team_owner.team.name, service_account.uuid) + response = api_client.delete(path=url, content_type="application/json") + + assert response.status_code == 204 + assert ServiceAccount.objects.filter(uuid=service_account.uuid).count() == 0 + + +@pytest.mark.django_db +def test_delete_service_account_fail_user_is_not_authenticated( + api_client: APIClient, + team: Team, + service_account: ServiceAccount, +): + assert ServiceAccount.objects.filter(uuid=service_account.uuid).count() == 1 + + url = get_delete_service_account_url(team.name, service_account.uuid) + response = api_client.delete(path=url, content_type="application/json") + expected_response = {"detail": "Authentication credentials were not provided."} + + assert response.status_code == 401 + assert response.json() == expected_response + assert ServiceAccount.objects.filter(uuid=service_account.uuid).count() == 1 + + +@pytest.mark.django_db +def test_delete_service_account_fails_because_user_is_not_team_member( + api_client: APIClient, + team: Team, + service_account: ServiceAccount, +): + assert ServiceAccount.objects.filter(uuid=service_account.uuid).count() == 1 + + non_team_user = User.objects.create() + api_client.force_authenticate(non_team_user) + + url = get_delete_service_account_url(team.name, service_account.uuid) + response = api_client.delete(path=url, content_type="application/json") + expected_response = {"non_field_errors": ["Must be a member to access team"]} + + assert response.status_code == 403 + assert response.json() == expected_response + assert ServiceAccount.objects.filter(uuid=service_account.uuid).count() == 1 + + +@pytest.mark.django_db +def test_delete_service_account_fail_because_user_is_not_team_owner( + api_client: APIClient, + team_member: TeamMember, + team: Team, + service_account: ServiceAccount, +): + assert ServiceAccount.objects.filter(uuid=service_account.uuid).count() == 1 + + api_client.force_authenticate(team_member.user) + url = get_delete_service_account_url(team.name, service_account.uuid) + response = api_client.delete(path=url, content_type="application/json") + expected_response = { + "non_field_errors": ["Must be an owner to delete a service account"] + } + + assert response.status_code == 403 + assert response.json() == expected_response + assert ServiceAccount.objects.filter(uuid=service_account.uuid).count() == 1 diff --git a/django/thunderstore/api/urls.py b/django/thunderstore/api/urls.py index 8289190d6..514f40796 100644 --- a/django/thunderstore/api/urls.py +++ b/django/thunderstore/api/urls.py @@ -5,6 +5,8 @@ CommunityAPIView, CommunityFiltersAPIView, CommunityListAPIView, + CreateServiceAccountAPIView, + DeleteServiceAccountAPIView, DeprecatePackageAPIView, DisbandTeamAPIView, PackageListingAPIView, @@ -170,4 +172,14 @@ TeamServiceAccountListAPIView.as_view(), name="cyberstorm.team.service-account", ), + path( + "team//service-account/create/", + CreateServiceAccountAPIView.as_view(), + name="cyberstorm.team.service-account.create", + ), + path( + "team//service-account/delete//", + DeleteServiceAccountAPIView.as_view(), + name="cyberstorm.team.service-account.delete", + ), ] From 12defc437d0adde622cf070cd878efc1dc9cea5c Mon Sep 17 00:00:00 2001 From: Roffenlund Date: Wed, 24 Sep 2025 18:39:15 +0300 Subject: [PATCH 24/61] Update service account form and views Update forms and views for create and delete service accounts. Refs. TS-2320 --- django/thunderstore/account/forms.py | 46 ++++++++++++------ .../account/tests/test_service_account.py | 48 ++++++++++++++++--- .../repository/views/team_settings.py | 12 ++++- 3 files changed, 83 insertions(+), 23 deletions(-) diff --git a/django/thunderstore/account/forms.py b/django/thunderstore/account/forms.py index 572a2934f..3e490415d 100644 --- a/django/thunderstore/account/forms.py +++ b/django/thunderstore/account/forms.py @@ -1,7 +1,12 @@ from django import forms +from django.core.exceptions import ValidationError from django.db import transaction from thunderstore.account.models import ServiceAccount +from thunderstore.api.cyberstorm.services.team import ( + create_service_account, + delete_service_account, +) from thunderstore.core.types import UserType from thunderstore.repository.models import Team @@ -16,19 +21,28 @@ def __init__(self, user: UserType, *args, **kwargs) -> None: queryset=Team.objects.filter(members__user=user, is_active=True), ) - def clean_team(self) -> Team: - team = self.cleaned_data["team"] - team.ensure_can_create_service_account(self.user) - return team - @transaction.atomic def save(self) -> ServiceAccount: + if self.errors: + raise ValueError("Cannot save form with errors") + + self.api_token = "" + owner = self.cleaned_data["team"] nickname = self.cleaned_data["nickname"] - (service_account, token) = ServiceAccount.create( - owner=owner, nickname=nickname, creator=self.user - ) - self.api_token = token + service_account = None + + try: + service_account, token = create_service_account( + agent=self.user, + team=owner, + nickname=nickname, + ) + self.api_token = token + except ValidationError as e: + self.add_error(None, e) + raise ValidationError(self.errors) + return service_account @@ -40,13 +54,17 @@ def __init__(self, user: UserType, *args, **kwargs) -> None: queryset=ServiceAccount.objects.filter(owner__members__user=user), ) - def clean_service_account(self) -> ServiceAccount: + def save(self) -> None: + if self.errors: + raise ValueError("Cannot save form with errors") + service_account = self.cleaned_data["service_account"] - service_account.owner.ensure_can_delete_service_account(self.user) - return service_account - def save(self) -> None: - self.cleaned_data["service_account"].delete() + try: + delete_service_account(self.user, service_account) + except ValidationError as e: + self.add_error(None, e) + raise ValidationError(self.errors) class EditServiceAccountForm(forms.Form): diff --git a/django/thunderstore/account/tests/test_service_account.py b/django/thunderstore/account/tests/test_service_account.py index bfe84c09a..11af6dad5 100644 --- a/django/thunderstore/account/tests/test_service_account.py +++ b/django/thunderstore/account/tests/test_service_account.py @@ -1,4 +1,5 @@ import pytest +from django.core.exceptions import ValidationError from rest_framework.authtoken.models import Token from thunderstore.account.forms import ( @@ -46,6 +47,24 @@ def test_service_account_create(user, team): ) +@pytest.mark.django_db +def test_service_account_create_error_on_save(user, team): + TeamMember.objects.create( + user=user, + team=team, + role=TeamMemberRole.owner, + ) + + form = CreateServiceAccountForm( + user, + data={"team": team, "nickname": "x" * 1000}, + ) + + assert form.is_valid() is False + with pytest.raises(ValueError): + form.save() + + @pytest.mark.django_db def test_service_account_create_nickname_too_long(user, team): TeamMember.objects.create( @@ -91,9 +110,11 @@ def test_service_account_create_not_owner(user, team): user, data={"team": team, "nickname": "Nickname"}, ) + with pytest.raises(ValidationError): + form.save() assert form.is_valid() is False - assert len(form.errors["team"]) == 1 - assert form.errors["team"][0] == "Must be an owner to create a service account" + assert len(form.errors["__all__"]) == 1 + assert form.errors["__all__"][0] == "Must be an owner to create a service account" @pytest.mark.django_db @@ -129,6 +150,20 @@ def test_service_account_delete_not_member(service_account): ) +@pytest.mark.django_db +def test_service_account_delete_error_on_save(service_account): + user = UserFactory.create() + + form = DeleteServiceAccountForm( + user, + data={"service_account": service_account}, + ) + + assert form.is_valid() is False + with pytest.raises(ValueError): + form.save() + + @pytest.mark.django_db def test_service_account_delete_not_owner(service_account): user = UserFactory.create() @@ -141,12 +176,11 @@ def test_service_account_delete_not_owner(service_account): user, data={"service_account": service_account}, ) + with pytest.raises(ValidationError): + form.save() assert form.is_valid() is False - assert len(form.errors["service_account"]) == 1 - assert ( - form.errors["service_account"][0] - == "Must be an owner to delete a service account" - ) + assert len(form.errors["__all__"]) == 1 + assert form.errors["__all__"][0] == "Must be an owner to delete a service account" @pytest.mark.django_db diff --git a/django/thunderstore/repository/views/team_settings.py b/django/thunderstore/repository/views/team_settings.py index 3bde18bb8..e77bafbde 100644 --- a/django/thunderstore/repository/views/team_settings.py +++ b/django/thunderstore/repository/views/team_settings.py @@ -240,7 +240,10 @@ def form_invalid(self, form): return super().form_invalid(form) def form_valid(self, form): - form.save() + try: + form.save() + except ValidationError: + return self.form_invalid(form) messages.success(self.request, "Action performed successfully") return redirect(self.object.service_accounts_url) @@ -255,11 +258,16 @@ def get_context_data(self, **kwargs): return context def form_valid(self, form): - form.save() + try: + form.save() + except ValidationError: + return self.form_invalid(form) + messages.success(self.request, "Service account added successfully") context = super().get_context_data() context["api_token"] = form.api_token context["nickname"] = form.cleaned_data["nickname"] + return render(self.request, self.template_name, context) From 38522a2f2d55e70dc1b3cdb2ac39aaf03574d907 Mon Sep 17 00:00:00 2001 From: Roffenlund Date: Thu, 25 Sep 2025 10:15:23 +0300 Subject: [PATCH 25/61] Add endpoints to endpoint tests Add create and delete endpoints to the endpoint test suite. Update the delete view swagger request_body parameter to display explicitly None. Refs. TS-2320 --- django/thunderstore/api/cyberstorm/tests/endpoint_data.py | 4 ++++ django/thunderstore/api/cyberstorm/views/team.py | 1 + 2 files changed, 5 insertions(+) diff --git a/django/thunderstore/api/cyberstorm/tests/endpoint_data.py b/django/thunderstore/api/cyberstorm/tests/endpoint_data.py index b0d5cbce6..444972152 100644 --- a/django/thunderstore/api/cyberstorm/tests/endpoint_data.py +++ b/django/thunderstore/api/cyberstorm/tests/endpoint_data.py @@ -42,6 +42,9 @@ "username": "TestUser", "role": "member", }, + "/api/cyberstorm/team/{team_name}/service-account/create/": { + "nickname": "TestServiceAccount", + }, }, "PATCH": { "/api/cyberstorm/team/{team_name}/update/": { @@ -51,6 +54,7 @@ "DELETE": [ "/api/cyberstorm/team/{team_name}/disband/", "/api/cyberstorm/team/{team_name}/member/{username}/remove/", + "/api/cyberstorm/team/{team_name}/service-account/delete/{uuid}/", ], } diff --git a/django/thunderstore/api/cyberstorm/views/team.py b/django/thunderstore/api/cyberstorm/views/team.py index c38d1bf57..399f0e9ac 100644 --- a/django/thunderstore/api/cyberstorm/views/team.py +++ b/django/thunderstore/api/cyberstorm/views/team.py @@ -203,6 +203,7 @@ class DeleteServiceAccountAPIView(APIView): permission_classes = [IsAuthenticated] @conditional_swagger_auto_schema( + request_body=None, operation_id="cyberstorm.team.service-account.delete", tags=["cyberstorm"], responses={status.HTTP_204_NO_CONTENT: ""}, From dd16ce9d64fca28b62d847adee98c3798c2a03a0 Mon Sep 17 00:00:00 2001 From: Roffenlund Date: Fri, 26 Sep 2025 17:30:33 +0300 Subject: [PATCH 26/61] Update test_team_services import Update the import location for UserFactory. Refs. TS-2320 --- .../api/cyberstorm/tests/services/test_team_services.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/django/thunderstore/api/cyberstorm/tests/services/test_team_services.py b/django/thunderstore/api/cyberstorm/tests/services/test_team_services.py index 526bfc1c0..7faf2fa77 100644 --- a/django/thunderstore/api/cyberstorm/tests/services/test_team_services.py +++ b/django/thunderstore/api/cyberstorm/tests/services/test_team_services.py @@ -3,9 +3,9 @@ from django.http import Http404 from conftest import TestUserTypes -from thunderstore.account.factories import UserFactory from thunderstore.api.cyberstorm.services import team as team_services from thunderstore.core.exceptions import PermissionValidationError +from thunderstore.core.factories import UserFactory from thunderstore.repository.models import Namespace, Team, TeamMemberRole from thunderstore.repository.models.team import TeamMember From f5607759e83df4383e791834d00b8644dbd08fbc Mon Sep 17 00:00:00 2001 From: Oksamies Date: Sat, 27 Sep 2025 01:09:56 +0300 Subject: [PATCH 27/61] Move few PackageListing serializers to shared files Move EmptyStringAsNoneField and CyberstormPackageTeamSerializer to shared serializers files, so that they can be cleanly used by PackageVersionAPIView --- .../api/cyberstorm/serializers/__init__.py | 4 +++ .../api/cyberstorm/serializers/package.py | 10 ++++++ .../api/cyberstorm/serializers/utils.py | 18 +++++++++++ .../api/cyberstorm/views/package_listing.py | 31 ++----------------- 4 files changed, 35 insertions(+), 28 deletions(-) create mode 100644 django/thunderstore/api/cyberstorm/serializers/utils.py diff --git a/django/thunderstore/api/cyberstorm/serializers/__init__.py b/django/thunderstore/api/cyberstorm/serializers/__init__.py index 83f560393..dfa79c924 100644 --- a/django/thunderstore/api/cyberstorm/serializers/__init__.py +++ b/django/thunderstore/api/cyberstorm/serializers/__init__.py @@ -6,6 +6,7 @@ from .package import ( CyberstormPackageDependencySerializer, CyberstormPackagePreviewSerializer, + CyberstormPackageTeamSerializer, PackagePermissionsSerializer, ) from .package_listing import PackageListingStatusResponseSerializer @@ -19,8 +20,10 @@ CyberstormTeamSerializer, CyberstormTeamUpdateSerializer, ) +from .utils import EmptyStringAsNoneField __all__ = [ + "EmptyStringAsNoneField", "CyberstormTeamAddMemberRequestSerializer", "CyberstormTeamAddMemberResponseSerializer", "CyberstormCreateTeamSerializer", @@ -29,6 +32,7 @@ "CyberstormPackageCategorySerializer", "CyberstormPackageListingSectionSerializer", "CyberstormPackagePreviewSerializer", + "CyberstormPackageTeamSerializer", "CyberstormServiceAccountSerializer", "CyberstormTeamMemberSerializer", "CyberstormTeamSerializer", diff --git a/django/thunderstore/api/cyberstorm/serializers/package.py b/django/thunderstore/api/cyberstorm/serializers/package.py index 000c325a7..5ed8ace4e 100644 --- a/django/thunderstore/api/cyberstorm/serializers/package.py +++ b/django/thunderstore/api/cyberstorm/serializers/package.py @@ -5,6 +5,7 @@ from thunderstore.api.cyberstorm.serializers.community import ( CyberstormPackageCategorySerializer, ) +from thunderstore.api.cyberstorm.serializers.team import CyberstormTeamMemberSerializer from thunderstore.community.models import Community from thunderstore.repository.models import Namespace, Package, PackageVersion @@ -77,3 +78,12 @@ def get_description(self, obj: PackageVersion) -> str: def get_icon_url(self, obj: PackageVersion) -> Optional[str]: return obj.icon.url if obj.is_effectively_active else None + + +class CyberstormPackageTeamSerializer(serializers.Serializer): + """ + Minimal information to present the team on package detail view. + """ + + name = serializers.CharField() + members = CyberstormTeamMemberSerializer(many=True, source="public_members") diff --git a/django/thunderstore/api/cyberstorm/serializers/utils.py b/django/thunderstore/api/cyberstorm/serializers/utils.py new file mode 100644 index 000000000..bb63943de --- /dev/null +++ b/django/thunderstore/api/cyberstorm/serializers/utils.py @@ -0,0 +1,18 @@ +from rest_framework import serializers + + +class EmptyStringAsNoneField(serializers.Field): + """ + Serialize empty string to None and deserialize vice versa. + """ + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.allow_null = True + self.allow_blank = True + + def to_representation(self, value): + return None if value == "" else value + + def to_internal_value(self, data): + return "" if data is None else data diff --git a/django/thunderstore/api/cyberstorm/views/package_listing.py b/django/thunderstore/api/cyberstorm/views/package_listing.py index 2e5fff828..00eb9a9e1 100644 --- a/django/thunderstore/api/cyberstorm/views/package_listing.py +++ b/django/thunderstore/api/cyberstorm/views/package_listing.py @@ -22,7 +22,8 @@ from thunderstore.api.cyberstorm.serializers import ( CyberstormPackageCategorySerializer, - CyberstormTeamMemberSerializer, + CyberstormPackageTeamSerializer, + EmptyStringAsNoneField, PackageListingStatusResponseSerializer, ) from thunderstore.api.cyberstorm.views.package_listing_actions import ( @@ -73,32 +74,6 @@ def get_is_unavailable(self, obj: PackageVersion) -> bool: return obj.version_is_unavailable -class TeamSerializer(serializers.Serializer): - """ - Minimal information to present the team on package detail view. - """ - - name = serializers.CharField() - members = CyberstormTeamMemberSerializer(many=True, source="public_members") - - -class EmptyStringAsNoneField(serializers.Field): - """ - Serialize empty string to None and deserialize vice versa. - """ - - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) - self.allow_null = True - self.allow_blank = True - - def to_representation(self, value): - return None if value == "" else value - - def to_internal_value(self, data): - return "" if data is None else data - - class ResponseSerializer(serializers.Serializer): """ Data shown on package detail view. @@ -132,7 +107,7 @@ class ResponseSerializer(serializers.Serializer): namespace = serializers.CharField(source="package.namespace.name") rating_count = serializers.IntegerField(min_value=0) size = serializers.IntegerField(min_value=0, source="package.latest.file_size") - team = TeamSerializer(source="package.owner") + team = CyberstormPackageTeamSerializer(source="package.owner") website_url = EmptyStringAsNoneField(source="package.latest.website_url") From 9fdd1cce3d403097d0a3c44c1ee557a4554ff174 Mon Sep 17 00:00:00 2001 From: Oksamies Date: Sat, 27 Sep 2025 01:10:52 +0300 Subject: [PATCH 28/61] Add PackageVersionAPIView and related serializers Add PackageVersionAPIView and related serializers. Currently dependencies are not included in the views output. There should be a separate view for dependencies implemented soon --- .../api/cyberstorm/serializers/__init__.py | 2 + .../cyberstorm/serializers/package_version.py | 29 +++++++ .../api/cyberstorm/tests/endpoint_data.py | 1 + .../cyberstorm/tests/test_package_version.py | 87 +++++++++++++++++++ .../api/cyberstorm/views/__init__.py | 2 + .../api/cyberstorm/views/package_version.py | 51 +++++++++++ django/thunderstore/api/urls.py | 6 ++ 7 files changed, 178 insertions(+) create mode 100644 django/thunderstore/api/cyberstorm/serializers/package_version.py create mode 100644 django/thunderstore/api/cyberstorm/tests/test_package_version.py create mode 100644 django/thunderstore/api/cyberstorm/views/package_version.py diff --git a/django/thunderstore/api/cyberstorm/serializers/__init__.py b/django/thunderstore/api/cyberstorm/serializers/__init__.py index dfa79c924..344cf49da 100644 --- a/django/thunderstore/api/cyberstorm/serializers/__init__.py +++ b/django/thunderstore/api/cyberstorm/serializers/__init__.py @@ -10,6 +10,7 @@ PackagePermissionsSerializer, ) from .package_listing import PackageListingStatusResponseSerializer +from .package_version import PackageVersionResponseSerializer from .team import ( CyberstormCreateServiceAccountSerializer, CyberstormCreateTeamSerializer, @@ -40,4 +41,5 @@ "CyberstormTeamUpdateSerializer", "CyberstormPackageDependencySerializer", "PackageListingStatusResponseSerializer", + "PackageVersionResponseSerializer", ] diff --git a/django/thunderstore/api/cyberstorm/serializers/package_version.py b/django/thunderstore/api/cyberstorm/serializers/package_version.py new file mode 100644 index 000000000..eceae1e62 --- /dev/null +++ b/django/thunderstore/api/cyberstorm/serializers/package_version.py @@ -0,0 +1,29 @@ +from rest_framework import serializers + +from thunderstore.api.cyberstorm.serializers.package import ( + CyberstormPackageTeamSerializer, +) +from thunderstore.api.cyberstorm.serializers.utils import EmptyStringAsNoneField + + +class PackageVersionResponseSerializer(serializers.Serializer): + """ + Data shown on package version detail view. + + Expects an annotated and customized CustomListing object. + """ + + datetime_created = serializers.DateTimeField(source="date_created") + dependency_count = serializers.IntegerField(min_value=0) + description = serializers.CharField() + download_count = serializers.IntegerField(source="downloads", min_value=0) + download_url = serializers.CharField(source="full_download_url") + full_version_name = serializers.CharField() + icon_url = serializers.CharField(source="icon.url") + install_url = serializers.CharField() + name = serializers.CharField() + version_number = serializers.CharField() + namespace = serializers.CharField(source="package.namespace.name") + size = serializers.IntegerField(min_value=0, source="file_size") + team = CyberstormPackageTeamSerializer(source="owner") + website_url = EmptyStringAsNoneField() diff --git a/django/thunderstore/api/cyberstorm/tests/endpoint_data.py b/django/thunderstore/api/cyberstorm/tests/endpoint_data.py index 444972152..05fd4a5bc 100644 --- a/django/thunderstore/api/cyberstorm/tests/endpoint_data.py +++ b/django/thunderstore/api/cyberstorm/tests/endpoint_data.py @@ -11,6 +11,7 @@ "/api/cyberstorm/package/{community_id}/{namespace_id}/{package_name}/permissions/", "/api/cyberstorm/package/{namespace_id}/{package_name}/latest/changelog/", "/api/cyberstorm/package/{namespace_id}/{package_name}/latest/readme/", + "/api/cyberstorm/package/{namespace_id}/{package_name}/v/{version_number}/", "/api/cyberstorm/package/{namespace_id}/{package_name}/v/{version_number}/changelog/", "/api/cyberstorm/package/{namespace_id}/{package_name}/v/{version_number}/readme/", "/api/cyberstorm/package/{namespace_id}/{package_name}/v/{version_number}/dependencies/", diff --git a/django/thunderstore/api/cyberstorm/tests/test_package_version.py b/django/thunderstore/api/cyberstorm/tests/test_package_version.py new file mode 100644 index 000000000..43d185e13 --- /dev/null +++ b/django/thunderstore/api/cyberstorm/tests/test_package_version.py @@ -0,0 +1,87 @@ +from datetime import datetime + +import pytest +from django.db import connection +from django.test.utils import CaptureQueriesContext +from rest_framework.test import APIClient + +from thunderstore.repository.factories import PackageVersionFactory, TeamMemberFactory +from thunderstore.repository.models.package_version import PackageVersion + + +def _date_to_z(value: datetime) -> str: + return value.strftime("%Y-%m-%dT%H:%M:%S.%fZ") + + +def _get_version_url(pv: PackageVersion) -> str: + return f"/api/cyberstorm/package/{pv.package.namespace.name}/{pv.name}/v/{pv.version_number}/" + + +# --------------------------------------------------------------------------- +# API view tests +# --------------------------------------------------------------------------- + + +@pytest.mark.django_db +def test_package_version_view__returns_info(api_client: APIClient) -> None: + pv1 = PackageVersionFactory( + downloads=12, + website_url="https://thunderstore.io/", + changelog="some changelog", + ) + pv2 = PackageVersionFactory() + pv3 = PackageVersionFactory() + pv4 = PackageVersionFactory() + + # Set multiple dependencies to ensure only direct dependencies are counted + pv1.dependencies.set([pv2, pv3]) + # Set a dependency that is not direct + pv2.dependencies.set([pv4]) + + TeamMemberFactory(team=pv1.package.owner, role="owner") + TeamMemberFactory(team=pv1.package.owner, role="member") + + url = _get_version_url(pv1) + response = api_client.get(url) + assert response.status_code == 200 + data = response.json() + + assert data["datetime_created"] == _date_to_z(pv1.date_created) + assert data["dependency_count"] == 2 + assert data["description"] == pv1.description + assert data["download_count"] == pv1.downloads + assert data["download_url"] == pv1.full_download_url + assert data["full_version_name"] == pv1.full_version_name + assert data["icon_url"].startswith("http") + assert data["name"] == pv1.name + assert data["namespace"] == pv1.package.namespace.name + assert data["size"] == pv1.file_size + assert data["team"]["name"] == pv1.package.owner.name + assert "members" in data["team"] + assert data["website_url"] == "https://thunderstore.io/" + + +@pytest.mark.django_db +def test_package_version_view__serializes_url_correctly(api_client: APIClient) -> None: + pv = PackageVersionFactory(website_url="https://thunderstore.io/") + url = _get_version_url(pv) + assert api_client.get(url).json()["website_url"] == "https://thunderstore.io/" + + pv.website_url = "" + pv.save(update_fields=("website_url",)) + assert api_client.get(url).json()["website_url"] is None + + +@pytest.mark.django_db +def test_package_version_view__query_count(api_client: APIClient) -> None: + pv = PackageVersionFactory() + deps = [PackageVersionFactory() for _ in range(10)] + pv.dependencies.set(deps) + url = _get_version_url(pv) + + with CaptureQueriesContext(connection) as ctx: + response = api_client.get(url) + + assert response.status_code == 200 + # Allow some overhead but ensure it's not exploding + assert len(ctx.captured_queries) < 20 diff --git a/django/thunderstore/api/cyberstorm/views/__init__.py b/django/thunderstore/api/cyberstorm/views/__init__.py index f2ea3e85e..5773db8a2 100644 --- a/django/thunderstore/api/cyberstorm/views/__init__.py +++ b/django/thunderstore/api/cyberstorm/views/__init__.py @@ -16,6 +16,7 @@ ) from .package_permissions import PackagePermissionsAPIView from .package_rating import RatePackageAPIView +from .package_version import PackageVersionAPIView from .package_version_list import ( PackageVersionDependenciesListAPIView, PackageVersionListAPIView, @@ -47,6 +48,7 @@ "PackageListingByDependencyListAPIView", "PackageListingByNamespaceListAPIView", "PackagePermissionsAPIView", + "PackageVersionAPIView", "PackageVersionChangelogAPIView", "PackageVersionDependenciesListAPIView", "PackageVersionListAPIView", diff --git a/django/thunderstore/api/cyberstorm/views/package_version.py b/django/thunderstore/api/cyberstorm/views/package_version.py new file mode 100644 index 000000000..5c0818946 --- /dev/null +++ b/django/thunderstore/api/cyberstorm/views/package_version.py @@ -0,0 +1,51 @@ +from django.db.models import Count +from django.http import HttpRequest +from rest_framework import status +from rest_framework.generics import get_object_or_404 +from rest_framework.response import Response +from rest_framework.views import APIView + +from thunderstore.api.cyberstorm.serializers.package_version import ( + PackageVersionResponseSerializer, +) +from thunderstore.api.utils import ( + CyberstormAutoSchemaMixin, + conditional_swagger_auto_schema, +) +from thunderstore.repository.models.package_version import PackageVersion + + +class PackageVersionAPIView(CyberstormAutoSchemaMixin, APIView): + @conditional_swagger_auto_schema( + responses={200: PackageVersionResponseSerializer}, + operation_id="cyberstorm.package_version", + tags=["cyberstorm"], + ) + def get( + self, + request: HttpRequest, + namespace_id: str, + package_name: str, + version_number: str, + ) -> Response: + instance = get_object_or_404( + PackageVersion.objects.filter(is_active=True) + .select_related( + "package", + "package__namespace", + ) + .prefetch_related( + "package__owner__members", + ) + .annotate( + dependency_count=Count( + "dependencies", + ) + ), + package__namespace__name=namespace_id, + name=package_name, + version_number=version_number, + ) + + response_serializer = PackageVersionResponseSerializer(instance=instance) + return Response(response_serializer.data, status=status.HTTP_200_OK) diff --git a/django/thunderstore/api/urls.py b/django/thunderstore/api/urls.py index 514f40796..9ce71768e 100644 --- a/django/thunderstore/api/urls.py +++ b/django/thunderstore/api/urls.py @@ -15,6 +15,7 @@ PackageListingByNamespaceListAPIView, PackageListingStatusAPIView, PackagePermissionsAPIView, + PackageVersionAPIView, PackageVersionChangelogAPIView, PackageVersionDependenciesListAPIView, PackageVersionListAPIView, @@ -97,6 +98,11 @@ PackageVersionReadmeAPIView.as_view(), name="cyberstorm.package.latest.readme", ), + path( + "package///v//", + PackageVersionAPIView.as_view(), + name="cyberstorm.package.version", + ), path( "package///v//changelog/", PackageVersionChangelogAPIView.as_view(), From 7ae1ccab6a7c99e248db8c82bd6580135b13ff03 Mon Sep 17 00:00:00 2001 From: Roffenlund Date: Thu, 25 Sep 2025 16:51:36 +0300 Subject: [PATCH 29/61] Implement remove user and social auth services Implement service for deleting a user from database. Implement service for removing a user's linked account. Ensure user cannot remove last linked social account. Implement tests. Refs. TS-2317 & TS-2318 --- django/conftest.py | 16 +++++++++ .../api/cyberstorm/services/user.py | 23 ++++++++++++ .../tests/services/test_user_services.py | 35 +++++++++++++++++++ 3 files changed, 74 insertions(+) create mode 100644 django/thunderstore/api/cyberstorm/services/user.py create mode 100644 django/thunderstore/api/cyberstorm/tests/services/test_user_services.py diff --git a/django/conftest.py b/django/conftest.py index 630e29f93..f92c1eeaf 100644 --- a/django/conftest.py +++ b/django/conftest.py @@ -14,6 +14,7 @@ from django.core.files.base import File from PIL import Image from rest_framework.test import APIClient +from social_django.models import UserSocialAuth from django_contracts.models import LegalContract, LegalContractVersion from thunderstore.account.factories import UserFlagFactory @@ -120,6 +121,21 @@ def user(django_user_model): ) +@pytest.fixture() +def user_with_social_auths(user): + UserSocialAuth.objects.create( + user=user, + provider="discord", + uid="1234567890", + ) + UserSocialAuth.objects.create( + user=user, + provider="github", + uid="0987654321", + ) + return user + + @pytest.fixture() def user_with_settings(django_user_model): user = django_user_model.objects.create_user( diff --git a/django/thunderstore/api/cyberstorm/services/user.py b/django/thunderstore/api/cyberstorm/services/user.py new file mode 100644 index 000000000..9b52b4517 --- /dev/null +++ b/django/thunderstore/api/cyberstorm/services/user.py @@ -0,0 +1,23 @@ +from django.db import transaction +from social_django.models import UserSocialAuth + +from thunderstore.core.exceptions import PermissionValidationError +from thunderstore.core.types import UserType + + +@transaction.atomic +def delete_user_account(target_user: UserType): + return target_user.delete() + + +@transaction.atomic +def delete_user_social_auth(social_auth: UserSocialAuth): + target_user = social_auth.user + + auth_qs = UserSocialAuth.objects.select_for_update().filter(user=target_user) + count = auth_qs.count() + + if count <= 1: + raise PermissionValidationError("Cannot disconnect last linked auth method") + + return auth_qs.filter(pk=social_auth.pk).delete() diff --git a/django/thunderstore/api/cyberstorm/tests/services/test_user_services.py b/django/thunderstore/api/cyberstorm/tests/services/test_user_services.py new file mode 100644 index 000000000..71390ac69 --- /dev/null +++ b/django/thunderstore/api/cyberstorm/tests/services/test_user_services.py @@ -0,0 +1,35 @@ +import pytest +from django.contrib.auth import get_user_model + +from thunderstore.api.cyberstorm.services.user import ( + delete_user_account, + delete_user_social_auth, +) +from thunderstore.core.exceptions import PermissionValidationError +from thunderstore.core.types import UserType + +User = get_user_model() + + +@pytest.mark.django_db +def test_delete_user_account_success(user: UserType): + assert User.objects.filter(username=user.username).exists() + delete_user_account(target_user=user) + assert not User.objects.filter(username=user.username).exists() + + +@pytest.mark.django_db +def test_delete_user_social_auth_success(user_with_social_auths: UserType): + social_auth = user_with_social_auths.social_auth.filter(provider="discord").first() + delete_user_social_auth(social_auth=social_auth) + assert not user_with_social_auths.social_auth.filter(provider="discord").exists() + + +@pytest.mark.django_db +def test_delete_user_social_auth_last_auth_method_raises_error( + user_with_social_auths: UserType, +): + user_with_social_auths.social_auth.filter(provider="discord").delete() + social_auth = user_with_social_auths.social_auth.first() + with pytest.raises(PermissionValidationError): + delete_user_social_auth(social_auth=social_auth) From 37a594eecb228173057e09e4fcfcf2701b3070a2 Mon Sep 17 00:00:00 2001 From: Roffenlund Date: Thu, 25 Sep 2025 16:53:59 +0300 Subject: [PATCH 30/61] Implement views for removing user & social account Implement views for deleting a user and deleting a user's linked social account. Refs. TS-2317 & TS-2318 --- .../api/cyberstorm/tests/test_user.py | 91 +++++++++++++++++++ .../api/cyberstorm/views/__init__.py | 3 + .../thunderstore/api/cyberstorm/views/user.py | 51 +++++++++++ django/thunderstore/api/urls.py | 12 +++ 4 files changed, 157 insertions(+) create mode 100644 django/thunderstore/api/cyberstorm/tests/test_user.py create mode 100644 django/thunderstore/api/cyberstorm/views/user.py diff --git a/django/thunderstore/api/cyberstorm/tests/test_user.py b/django/thunderstore/api/cyberstorm/tests/test_user.py new file mode 100644 index 000000000..059aef921 --- /dev/null +++ b/django/thunderstore/api/cyberstorm/tests/test_user.py @@ -0,0 +1,91 @@ +import pytest +from django.contrib.auth import get_user_model +from rest_framework.test import APIClient + +from thunderstore.core.types import UserType + +User = get_user_model() + + +def get_delete_user_url() -> str: + return "/api/cyberstorm/user/delete/" + + +def get_disconnect_user_linked_account_url(provider: str) -> str: + return f"/api/cyberstorm/user/linked-account/{provider}/disconnect/" + + +@pytest.mark.django_db +def test_delete_user_success(user: UserType, api_client: APIClient): + api_client.force_authenticate(user=user) + + assert User.objects.filter(username=user.username).exists() + url = get_delete_user_url() + response = api_client.delete(url, content_type="application/json") + assert response.status_code == 204 + assert not User.objects.filter(username=user.username).exists() + + +@pytest.mark.django_db +def test_delete_user_fail_unauthenticated(api_client: APIClient): + url = get_delete_user_url() + response = api_client.delete(url, content_type="application/json") + assert response.status_code == 401 + + +@pytest.mark.django_db +def test_disconnect_user_linked_account_success( + user_with_social_auths: UserType, api_client: APIClient +): + api_client.force_authenticate(user=user_with_social_auths) + assert user_with_social_auths.social_auth.filter(provider="discord").exists() + + url = get_disconnect_user_linked_account_url("discord") + response = api_client.delete(url, content_type="application/json") + + assert response.status_code == 204 + assert not user_with_social_auths.social_auth.filter(provider="discord").exists() + + +@pytest.mark.django_db +def test_disconnect_user_linked_account_fail_unauthenticated(api_client: APIClient): + url = get_disconnect_user_linked_account_url("discord") + response = api_client.delete(url, content_type="application/json") + assert response.status_code == 401 + + +@pytest.mark.django_db +def test_disconnect_user_non_existent_linked_account( + user_with_social_auths: UserType, api_client: APIClient +): + api_client.force_authenticate(user=user_with_social_auths) + url = get_disconnect_user_linked_account_url("non-existent") + response = api_client.delete(url, content_type="application/json") + assert response.status_code == 404 + assert response.json() == {"detail": "Not found."} + + +@pytest.mark.django_db +def test_disconnect_user_with_no_social_auth(user: UserType, api_client: APIClient): + api_client.force_authenticate(user=user) + url = get_disconnect_user_linked_account_url("non-existent") + response = api_client.delete(url, content_type="application/json") + assert response.status_code == 404 + assert response.json() == {"detail": "Not found."} + + +@pytest.mark.django_db +def test_disconnect_user_linked_account_fail_last_linked_account( + user_with_social_auths: UserType, api_client: APIClient +): + api_client.force_authenticate(user=user_with_social_auths) + user_with_social_auths.social_auth.filter(provider="discord").delete() + url = get_disconnect_user_linked_account_url("github") + + response = api_client.delete(url, content_type="application/json") + expected_response = { + "non_field_errors": ["Cannot disconnect last linked auth method"] + } + + assert response.status_code == 403 + assert response.json() == expected_response diff --git a/django/thunderstore/api/cyberstorm/views/__init__.py b/django/thunderstore/api/cyberstorm/views/__init__.py index f2ea3e85e..07fb81b93 100644 --- a/django/thunderstore/api/cyberstorm/views/__init__.py +++ b/django/thunderstore/api/cyberstorm/views/__init__.py @@ -32,6 +32,7 @@ TeamServiceAccountListAPIView, UpdateTeamAPIView, ) +from .user import DeleteUserAPIView, DisconnectUserLinkedAccountAPIView __all__ = [ "CommunityAPIView", @@ -39,6 +40,8 @@ "CommunityListAPIView", "CreateServiceAccountAPIView", "DeleteServiceAccountAPIView", + "DeleteUserAPIView", + "DisconnectUserLinkedAccountAPIView", "DeprecatePackageAPIView", "DisbandTeamAPIView", "PackageListingAPIView", diff --git a/django/thunderstore/api/cyberstorm/views/user.py b/django/thunderstore/api/cyberstorm/views/user.py new file mode 100644 index 000000000..6eb33d9fa --- /dev/null +++ b/django/thunderstore/api/cyberstorm/views/user.py @@ -0,0 +1,51 @@ +from django.contrib.auth import get_user_model +from django.http import Http404 +from django.utils.decorators import method_decorator +from rest_framework import status +from rest_framework.permissions import IsAuthenticated +from rest_framework.response import Response +from rest_framework.views import APIView + +from thunderstore.api.cyberstorm.services.user import ( + delete_user_account, + delete_user_social_auth, +) +from thunderstore.api.utils import conditional_swagger_auto_schema + +User = get_user_model() + + +@method_decorator( + name="delete", + decorator=conditional_swagger_auto_schema( + responses={status.HTTP_204_NO_CONTENT: None}, + operation_id="cyberstorm.user.delete", + tags=["cyberstorm"], + ), +) +class DeleteUserAPIView(APIView): + permission_classes = [IsAuthenticated] + + def delete(self, request, *args, **kwargs): + delete_user_account(target_user=request.user) + return Response(status=status.HTTP_204_NO_CONTENT) + + +@method_decorator( + name="delete", + decorator=conditional_swagger_auto_schema( + responses={status.HTTP_204_NO_CONTENT: None}, + operation_id="cyberstorm.user.linked_account.disconnect", + tags=["cyberstorm"], + ), +) +class DisconnectUserLinkedAccountAPIView(APIView): + permission_classes = [IsAuthenticated] + + def delete(self, request, provider: str, *args, **kwargs): + social_auth = request.user.social_auth.filter(provider=provider).first() + if not social_auth: + raise Http404("No linked account found") + + delete_user_social_auth(social_auth=social_auth) + return Response(status=status.HTTP_204_NO_CONTENT) diff --git a/django/thunderstore/api/urls.py b/django/thunderstore/api/urls.py index 514f40796..a20478ae4 100644 --- a/django/thunderstore/api/urls.py +++ b/django/thunderstore/api/urls.py @@ -7,8 +7,10 @@ CommunityListAPIView, CreateServiceAccountAPIView, DeleteServiceAccountAPIView, + DeleteUserAPIView, DeprecatePackageAPIView, DisbandTeamAPIView, + DisconnectUserLinkedAccountAPIView, PackageListingAPIView, PackageListingByCommunityListAPIView, PackageListingByDependencyListAPIView, @@ -182,4 +184,14 @@ DeleteServiceAccountAPIView.as_view(), name="cyberstorm.team.service-account.delete", ), + path( + "user/delete/", + DeleteUserAPIView.as_view(), + name="cyberstorm.user.delete", + ), + path( + "user/linked-account//disconnect/", + DisconnectUserLinkedAccountAPIView.as_view(), + name="cyberstorm.user.linked-account.disconnect", + ), ] From 8ad5d57ee4324e91cd6d4e8857f8ba35caedb2ad Mon Sep 17 00:00:00 2001 From: Roffenlund Date: Thu, 25 Sep 2025 16:55:03 +0300 Subject: [PATCH 31/61] Update form views and forms Update the forms and form views for removing user and removing user's social accounts. Implement tests. Update template to displaty error. A user should not be able to remove the last social auth account but it can be bypassed by removing the disabled attribute with deveoper tools. Refs. TS-2317 & TS-2318 --- .../templates/settings/linked_accounts.html | 3 + .../social/tests/test_user_forms.py | 72 ++++++++++ .../social/tests/test_user_views.py | 132 ++++++++++++++++++ django/thunderstore/social/views.py | 37 +++-- 4 files changed, 236 insertions(+), 8 deletions(-) create mode 100644 django/thunderstore/social/tests/test_user_forms.py create mode 100644 django/thunderstore/social/tests/test_user_views.py diff --git a/django/thunderstore/social/templates/settings/linked_accounts.html b/django/thunderstore/social/templates/settings/linked_accounts.html index 64d609c8e..9d4afef38 100644 --- a/django/thunderstore/social/templates/settings/linked_accounts.html +++ b/django/thunderstore/social/templates/settings/linked_accounts.html @@ -4,6 +4,9 @@ {% block title %}Linked Accounts{% endblock %} {% block settings_content %} +
+ {{ form.non_field_errors }} +
{% for entry in backends.associated|dictsort:"provider" %}
diff --git a/django/thunderstore/social/tests/test_user_forms.py b/django/thunderstore/social/tests/test_user_forms.py new file mode 100644 index 000000000..3db79f755 --- /dev/null +++ b/django/thunderstore/social/tests/test_user_forms.py @@ -0,0 +1,72 @@ +import pytest +from django.contrib.auth import get_user_model +from django.core.exceptions import ValidationError +from django.http import Http404 + +from thunderstore.social.views import DeleteAccountForm, LinkedAccountDisconnectForm + +User = get_user_model() + + +@pytest.mark.django_db +@pytest.mark.parametrize( + "test_data, success", [("github", True), ("", False), (None, False)] +) +def test_linked_account_disconnect_form_validation(test_data, success): + form = LinkedAccountDisconnectForm(data={"provider": test_data}) + if success: + assert form.is_valid() + else: + + assert not form.is_valid() + assert "provider" in form.errors + + +@pytest.mark.django_db +@pytest.mark.parametrize( + "test_data, success", [("github", True), ("", False), (None, False)] +) +def test_linked_account_disconnect_form_disconnect_account( + user_with_social_auths, test_data, success +): + form = LinkedAccountDisconnectForm(data={"provider": test_data}) + + if success: + assert form.is_valid() + form.disconnect_account(test_data, user_with_social_auths) + assert form.errors == {} + assert not user_with_social_auths.social_auth.filter( + provider=test_data + ).exists() + else: + with pytest.raises(Http404, match="Social auth not found"): + form.disconnect_account(test_data, user_with_social_auths) + + +@pytest.mark.django_db +def test_linked_account_disconnect_form_disconnect_last_auth_method( + user_with_social_auths, +): + user_with_social_auths.social_auth.filter(provider="discord").delete() + form = LinkedAccountDisconnectForm(data={"provider": "github"}) + with pytest.raises(ValidationError): + form.disconnect_account("github", user_with_social_auths) + assert form.errors == {"__all__": ["Cannot disconnect last linked auth method"]} + + +@pytest.mark.django_db +def test_delete_account_form_validation(user): + form = DeleteAccountForm(data={"verification": user.username}, user=user) + assert form.is_valid() + + form = DeleteAccountForm(data={"verification": "wrong username"}, user=user) + assert not form.is_valid() + assert "verification" in form.errors + + +@pytest.mark.django_db +def test_delete_account_form_delete_user(user): + form = DeleteAccountForm(data={"verification": user.username}, user=user) + assert form.is_valid() + form.delete_user() + assert not User.objects.filter(username=user.username).exists() diff --git a/django/thunderstore/social/tests/test_user_views.py b/django/thunderstore/social/tests/test_user_views.py new file mode 100644 index 000000000..8d3c48f40 --- /dev/null +++ b/django/thunderstore/social/tests/test_user_views.py @@ -0,0 +1,132 @@ +import pytest +from django.test import Client +from django.urls import reverse_lazy + +from thunderstore.community.models import CommunitySite +from thunderstore.core.types import UserType + + +@pytest.mark.django_db +@pytest.mark.parametrize( + "provider, should_fail", [("discord", False), ("non-existent", True)] +) +def test_disconnect_account( + client: Client, + community_site: CommunitySite, + user_with_social_auths: UserType, + provider: str, + should_fail: bool, +): + client.force_login(user_with_social_auths) + + url = reverse_lazy("settings.linked-accounts") + response = client.post( + url, + HTTP_HOST=community_site.site.domain, + data={"provider": provider}, + follow=True, + ) + + if should_fail: + assert response.status_code == 404 + assert response.context["exception"].args == ("Social auth not found",) + else: + assert response.status_code == 200 + assert not user_with_social_auths.social_auth.filter(provider=provider).exists() + + +@pytest.mark.django_db +def test_disconnect_account_unauthenticated( + client: Client, + community_site: CommunitySite, +): + url = reverse_lazy("settings.linked-accounts") + response = client.post( + url, + HTTP_HOST=community_site.site.domain, + data={"provider": "discord"}, + follow=True, + ) + assert response.request["PATH_INFO"] == "/" # redirects back to index + + +@pytest.mark.django_db +def test_disconnect_account_cannot_disconnect( + client: Client, + community_site: CommunitySite, + user_with_social_auths: UserType, +): + client.force_login(user_with_social_auths) + + user_with_social_auths.social_auth.filter(provider="discord").delete() + + url = reverse_lazy("settings.linked-accounts") + response = client.post( + url, + HTTP_HOST=community_site.site.domain, + data={"provider": "github"}, + follow=True, + ) + + assert response.status_code == 200 + assert user_with_social_auths.social_auth.filter(provider="github").exists() + assert response.context["form"].errors == { + "__all__": ["Cannot disconnect last linked auth method"] + } + + +@pytest.mark.django_db +def test_delete_account_success( + client: Client, + community_site: CommunitySite, + user: UserType, +): + client.force_login(user) + user_pk = user.pk + + url = reverse_lazy("settings.delete-account") + response = client.post( + url, + HTTP_HOST=community_site.site.domain, + data={"verification": user.username}, + follow=True, + ) + + assert response.status_code == 200 + assert not user.__class__.objects.filter(pk=user_pk).exists() + + +@pytest.mark.django_db +def test_delete_account_invalid_verification( + client: Client, + community_site: CommunitySite, + user: UserType, +): + client.force_login(user) + + url = reverse_lazy("settings.delete-account") + response = client.post( + url, + HTTP_HOST=community_site.site.domain, + data={"verification": "wrongusername"}, + follow=True, + ) + + assert response.status_code == 200 + assert response.context["form"].errors == {"verification": ["Invalid verification"]} + assert user.__class__.objects.filter(pk=user.pk).exists() + + +@pytest.mark.django_db +def test_delete_account_unauthenticated( + client: Client, + community_site: CommunitySite, +): + url = reverse_lazy("settings.delete-account") + response = client.post( + url, + HTTP_HOST=community_site.site.domain, + data={"verification": "testuser"}, + follow=True, + ) + assert response.request["PATH_INFO"] == "/" diff --git a/django/thunderstore/social/views.py b/django/thunderstore/social/views.py index b6776fdbd..3ca057dd3 100644 --- a/django/thunderstore/social/views.py +++ b/django/thunderstore/social/views.py @@ -1,8 +1,16 @@ from django import forms +from django.core.exceptions import ValidationError +from django.db import transaction +from django.http import Http404 from django.urls import reverse_lazy from django.views.generic.edit import FormView +from thunderstore.api.cyberstorm.services.user import ( + delete_user_account, + delete_user_social_auth, +) from thunderstore.core.mixins import RequireAuthenticationMixin +from thunderstore.core.types import UserType from thunderstore.frontend.views import SettingsViewMixin from thunderstore.repository.models import TeamMember @@ -10,6 +18,18 @@ class LinkedAccountDisconnectForm(forms.Form): provider = forms.CharField() + @transaction.atomic + def disconnect_account(self, provider: str, user: UserType): + social_auth = user.social_auth.filter(provider=provider).first() + if not social_auth: + raise Http404("Social auth not found") + + try: + delete_user_social_auth(social_auth=social_auth) + except ValidationError as e: + self.add_error(None, e) + raise ValidationError(self.errors) + class LinkedAccountsView(SettingsViewMixin, RequireAuthenticationMixin, FormView): template_name = "settings/linked_accounts.html" @@ -26,14 +46,11 @@ def get_context_data(self, **kwargs): def can_disconnect(self): return self.request.user.social_auth.count() > 1 - def disconnect_account(self, provider): - if not self.can_disconnect: - return - social_auth = self.request.user.social_auth.filter(provider=provider).first() - social_auth.delete() - def form_valid(self, form): - self.disconnect_account(form.cleaned_data["provider"]) + try: + form.disconnect_account(form.cleaned_data["provider"], self.request.user) + except ValidationError: + return self.form_invalid(form) return super().form_valid(form) @@ -50,6 +67,10 @@ def clean_verification(self): raise forms.ValidationError("Invalid verification") return data + @transaction.atomic + def delete_user(self): + delete_user_account(target_user=self.user) + class DeleteAccountView(SettingsViewMixin, RequireAuthenticationMixin, FormView): template_name = "settings/delete_account.html" @@ -73,5 +94,5 @@ def get_form_kwargs(self, *args, **kwargs): return kwargs def form_valid(self, form): - self.request.user.delete() + form.delete_user() return super().form_valid(form) From cfb09ba26fc976e0fcaa5e61b036e5778597bcac Mon Sep 17 00:00:00 2001 From: Roffenlund Date: Fri, 26 Sep 2025 15:29:49 +0300 Subject: [PATCH 32/61] Add endpoints to endpoint tests Update test utils to add social auth information to test user. Refs. TS-2317 & TS-2318 --- .../api/cyberstorm/tests/endpoint_data.py | 2 ++ django/thunderstore/api/cyberstorm/tests/utils.py | 13 +++++++++++++ 2 files changed, 15 insertions(+) diff --git a/django/thunderstore/api/cyberstorm/tests/endpoint_data.py b/django/thunderstore/api/cyberstorm/tests/endpoint_data.py index 444972152..85d57fb9a 100644 --- a/django/thunderstore/api/cyberstorm/tests/endpoint_data.py +++ b/django/thunderstore/api/cyberstorm/tests/endpoint_data.py @@ -55,6 +55,8 @@ "/api/cyberstorm/team/{team_name}/disband/", "/api/cyberstorm/team/{team_name}/member/{username}/remove/", "/api/cyberstorm/team/{team_name}/service-account/delete/{uuid}/", + "/api/cyberstorm/user/delete/", + "/api/cyberstorm/user/linked-account/{provider}/disconnect/", ], } diff --git a/django/thunderstore/api/cyberstorm/tests/utils.py b/django/thunderstore/api/cyberstorm/tests/utils.py index 7f34a870f..f30f9b0d7 100644 --- a/django/thunderstore/api/cyberstorm/tests/utils.py +++ b/django/thunderstore/api/cyberstorm/tests/utils.py @@ -5,6 +5,7 @@ from django.test.utils import CaptureQueriesContext from jsonschema import RefResolver, ValidationError, validate from rest_framework.test import APIClient +from social_django.models import UserSocialAuth from thunderstore.core.factories import UserFactory from thunderstore.repository.models import PackageListing, TeamMemberRole @@ -23,6 +24,7 @@ def get_parameter_values( "team_id": package_listing.package.owner.name, "team_name": package_listing.package.owner.name, "uuid": service_account.uuid if service_account else "", + "provider": "discord", } if username: @@ -31,8 +33,19 @@ def get_parameter_values( return parameters +def _add_social_auth_to_user(user): + providers = ["discord", "github"] + for provider in providers: + UserSocialAuth.objects.create( + user=user, + provider=provider, + uid=f"1234567890-{provider}", + ) + + def setup_superuser_with_package(package_listing, package_category=None): user = UserFactory.create(is_superuser=True) + _add_social_auth_to_user(user) UserFactory.create(username="TestUser", email="test@user.dev", is_active=True) From 623b935da687c45ab31b70066462440a1250af31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antti=20M=C3=A4ki?= Date: Tue, 30 Sep 2025 16:56:41 +0300 Subject: [PATCH 33/61] Use custom decorator to hide Cyberstorm endpoints from API docs --- .../api/cyberstorm/views/package_deprecate.py | 4 ++-- .../api/cyberstorm/views/package_listing_actions.py | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/django/thunderstore/api/cyberstorm/views/package_deprecate.py b/django/thunderstore/api/cyberstorm/views/package_deprecate.py index 36742844f..89a4c3d24 100644 --- a/django/thunderstore/api/cyberstorm/views/package_deprecate.py +++ b/django/thunderstore/api/cyberstorm/views/package_deprecate.py @@ -1,5 +1,4 @@ from django.shortcuts import get_object_or_404 -from drf_yasg.utils import swagger_auto_schema from rest_framework import serializers, status from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response @@ -9,6 +8,7 @@ deprecate_package, undeprecate_package, ) +from thunderstore.api.utils import conditional_swagger_auto_schema from thunderstore.repository.models import Package @@ -23,7 +23,7 @@ class SimpleSuccessResponseSerializer(serializers.Serializer): class DeprecatePackageAPIView(APIView): permission_classes = [IsAuthenticated] - @swagger_auto_schema( + @conditional_swagger_auto_schema( operation_id="cyberstorm.package.deprecate", request_body=DeprecatePackageSerializer, tags=["cyberstorm"], diff --git a/django/thunderstore/api/cyberstorm/views/package_listing_actions.py b/django/thunderstore/api/cyberstorm/views/package_listing_actions.py index 5552abc4a..5318c6490 100644 --- a/django/thunderstore/api/cyberstorm/views/package_listing_actions.py +++ b/django/thunderstore/api/cyberstorm/views/package_listing_actions.py @@ -1,5 +1,4 @@ from django.shortcuts import get_object_or_404 -from drf_yasg.utils import swagger_auto_schema from rest_framework import status from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response @@ -16,6 +15,7 @@ reject_package_listing, update_categories, ) +from thunderstore.api.utils import conditional_swagger_auto_schema from thunderstore.repository.models import PackageListing @@ -42,7 +42,7 @@ class UpdatePackageListingCategoriesAPIView(APIView): permission_classes = [IsAuthenticated] serializer_class = PackageListingUpdateSerializer - @swagger_auto_schema( + @conditional_swagger_auto_schema( operation_id="cyberstorm.package_listing.update", request_body=serializer_class, responses={200: PackageListingCategoriesSerializer}, @@ -70,7 +70,7 @@ class RejectPackageListingAPIView(APIView): permission_classes = [IsAuthenticated] serializer_class = PackageListingRejectSerializer - @swagger_auto_schema( + @conditional_swagger_auto_schema( operation_id="cyberstorm.package_listing.reject", request_body=serializer_class, responses={200: "Success"}, @@ -100,7 +100,7 @@ class ApprovePackageListingAPIView(APIView): permission_classes = [IsAuthenticated] serializer_class = PackageListingApproveSerializer - @swagger_auto_schema( + @conditional_swagger_auto_schema( operation_id="cyberstorm.package_listing.approve", request_body=serializer_class, responses={200: "Success"}, From 99eb8fc09760069152364549819af89cdb50006f Mon Sep 17 00:00:00 2001 From: Daniel Vainio Date: Wed, 1 Oct 2025 13:43:06 +0300 Subject: [PATCH 34/61] Implement report package view in cyberstorm API Implement and APIView which is responsible for reporting packages. Utilize existing service. Utilize existing serializer from the Experimental API. Refs. TS-2638 --- .../api/cyberstorm/views/__init__.py | 2 ++ .../views/package_listing_actions.py | 35 +++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/django/thunderstore/api/cyberstorm/views/__init__.py b/django/thunderstore/api/cyberstorm/views/__init__.py index 56179c8e5..1988749b7 100644 --- a/django/thunderstore/api/cyberstorm/views/__init__.py +++ b/django/thunderstore/api/cyberstorm/views/__init__.py @@ -7,6 +7,7 @@ from .package_listing_actions import ( ApprovePackageListingAPIView, RejectPackageListingAPIView, + ReportPackageListingAPIView, UpdatePackageListingCategoriesAPIView, ) from .package_listing_list import ( @@ -67,4 +68,5 @@ "RejectPackageListingAPIView", "ApprovePackageListingAPIView", "UpdateTeamAPIView", + "ReportPackageListingAPIView", ] diff --git a/django/thunderstore/api/cyberstorm/views/package_listing_actions.py b/django/thunderstore/api/cyberstorm/views/package_listing_actions.py index 5318c6490..1a439c171 100644 --- a/django/thunderstore/api/cyberstorm/views/package_listing_actions.py +++ b/django/thunderstore/api/cyberstorm/views/package_listing_actions.py @@ -13,9 +13,13 @@ from thunderstore.api.cyberstorm.services.package_listing import ( approve_package_listing, reject_package_listing, + report_package_listing, update_categories, ) from thunderstore.api.utils import conditional_swagger_auto_schema +from thunderstore.community.api.experimental.serializers import ( + PackageListingReportRequestSerializer, +) from thunderstore.repository.models import PackageListing @@ -123,3 +127,34 @@ def post(self, request, *args, **kwargs) -> Response: ) return Response({"message": "Success"}, status=status.HTTP_200_OK) + + +class ReportPackageListingAPIView(APIView): + permission_classes = [IsAuthenticated] + + @conditional_swagger_auto_schema( + operation_id="cyberstorm.package_listing.report", + tags=["cyberstorm"], + request_body=PackageListingReportRequestSerializer, + responses={200: "Success"}, + ) + def post(self, request, *args, **kwargs) -> Response: + listing: PackageListing = get_package_listing( + namespace_id=kwargs["namespace_id"], + package_name=kwargs["package_name"], + community_id=kwargs["community_id"], + ) + + request_serializer = PackageListingReportRequestSerializer(data=request.data) + request_serializer.is_valid(raise_exception=True) + + report_package_listing( + agent=request.user, + reason=request_serializer.validated_data.get("reason"), + package=listing.package, + package_listing=listing, + package_version=request_serializer.validated_data.get("version"), + description=request_serializer.validated_data.get("description"), + ) + + return Response({"message": "Success"}, status=status.HTTP_200_OK) From 83864e74727a624cdffd39981b095207ba247b0a Mon Sep 17 00:00:00 2001 From: Daniel Vainio Date: Wed, 1 Oct 2025 13:44:45 +0300 Subject: [PATCH 35/61] Implement endpoint for reporting packages Implement a new endpoint in the cyberstorm API which handles reporting packages. Implement tests. Refactor test file. Refs. TS-2638 --- .../api/cyberstorm/tests/endpoint_data.py | 3 + .../tests/test_package_listing_actions.py | 169 ++++++++++-------- django/thunderstore/api/urls.py | 6 + 3 files changed, 102 insertions(+), 76 deletions(-) diff --git a/django/thunderstore/api/cyberstorm/tests/endpoint_data.py b/django/thunderstore/api/cyberstorm/tests/endpoint_data.py index 3d1569d86..473e32751 100644 --- a/django/thunderstore/api/cyberstorm/tests/endpoint_data.py +++ b/django/thunderstore/api/cyberstorm/tests/endpoint_data.py @@ -30,6 +30,9 @@ "/api/cyberstorm/listing/{community_id}/{namespace_id}/{package_name}/update/": { "categories": ["test"], }, + "/api/cyberstorm/listing/{community_id}/{namespace_id}/{package_name}/report/": { + "reason": "Spam", + }, "/api/cyberstorm/package/{namespace_id}/{package_name}/deprecate/": { "deprecate": True, }, diff --git a/django/thunderstore/api/cyberstorm/tests/test_package_listing_actions.py b/django/thunderstore/api/cyberstorm/tests/test_package_listing_actions.py index 082dad792..5d5469ffa 100644 --- a/django/thunderstore/api/cyberstorm/tests/test_package_listing_actions.py +++ b/django/thunderstore/api/cyberstorm/tests/test_package_listing_actions.py @@ -27,25 +27,8 @@ def get_reject_url(package_listing): return f"{get_base_url(package_listing)}/reject/" -def perform_404_test( - api_client: APIClient, - active_package_listing: PackageListing, - user: UserType, - url: str, -): - active_package_listing.package.owner.add_member(user, role="owner") - api_client.force_authenticate(user=user) - - data = json.dumps( - { - "rejection_reason": "Invalid content", - "internal_notes": "Some internal notes", - } - ) - - response = api_client.post(url, data=data, content_type="application/json") - assert response.status_code == 404 - assert response.json() == {"detail": "Not found."} +def get_report_url(package_listing): + return f"{get_base_url(package_listing)}/report/" def perform_package_listing_action_test( @@ -54,6 +37,7 @@ def perform_package_listing_action_test( user_type: str, url: str, data: dict, + expected_status_code_map: dict = None, ): user = TestUserTypes.get_user_by_type(user_type) @@ -68,28 +52,64 @@ def perform_package_listing_action_test( package_listing.refresh_from_db() - expected_status_code = { - TestUserTypes.no_user: 401, - TestUserTypes.unauthenticated: 401, - TestUserTypes.regular_user: 403, - TestUserTypes.deactivated_user: 403, - TestUserTypes.service_account: 403, - TestUserTypes.site_admin: 200, - TestUserTypes.superuser: 200, - } + if not expected_status_code_map: + expected_status_code_map = { + TestUserTypes.no_user: 401, + TestUserTypes.unauthenticated: 401, + TestUserTypes.regular_user: 403, + TestUserTypes.deactivated_user: 403, + TestUserTypes.service_account: 403, + TestUserTypes.site_admin: 200, + TestUserTypes.superuser: 200, + } - assert response.status_code == expected_status_code[user_type] + assert response.status_code == expected_status_code_map[user_type] + + +@pytest.mark.django_db +@pytest.mark.parametrize("url_action", ["update", "approve", "reject", "report"]) +@pytest.mark.parametrize( + "invalid_field", ["community_id", "namespace_id", "package_name"] +) +def test_action_endpoints_404( + url_action: str, + invalid_field: str, + api_client: APIClient, + active_package_listing: PackageListing, + user: UserType, +): + community_id = active_package_listing.community.identifier + namespace_id = active_package_listing.package.namespace.name + package_name = active_package_listing.package.name + + if invalid_field == "community_id": + community_id = "invalid-id" + elif invalid_field == "namespace_id": + namespace_id = "invalid-id" + elif invalid_field == "package_name": + package_name = "invalid-name" + + url = ( + f"/api/cyberstorm/listing/" + f"{community_id}/{namespace_id}/{package_name}/{url_action}/" + ) + + active_package_listing.package.owner.add_member(user, role="owner") + api_client.force_authenticate(user=user) + + response = api_client.post(url, data={}, content_type="application/json") + assert response.status_code == 404 + assert response.json() == {"detail": "Not found."} @pytest.mark.django_db @pytest.mark.parametrize("user_type", TestUserTypes.options()) -def test_update_categories_success( +def test_update_package_listing( active_package_listing: PackageListing, api_client: APIClient, package_category: PackageCategory, user_type: str, ): - perform_package_listing_action_test( api_client=api_client, package_listing=active_package_listing, @@ -106,7 +126,6 @@ def test_reject_package_listing( active_package_listing: PackageListing, user_type: str, ): - perform_package_listing_action_test( api_client=api_client, package_listing=active_package_listing, @@ -149,50 +168,6 @@ def test_approve_package_listing( ) -@pytest.mark.django_db -@pytest.mark.parametrize("url_action", ["update", "approve", "reject"]) -def test_get_community_404( - url_action: str, - api_client: APIClient, - active_package_listing: PackageListing, - user: UserType, -): - url = ( - f"/api/cyberstorm/listing/invalid_community_id/" - f"{active_package_listing.package.namespace.name}/" - f"{active_package_listing.package.name}/{url_action}/" - ) - - perform_404_test( - api_client=api_client, - active_package_listing=active_package_listing, - user=user, - url=url, - ) - - -@pytest.mark.django_db -@pytest.mark.parametrize("url_action", ["update", "approve", "reject"]) -def test_get_package_404( - url_action: str, - api_client: APIClient, - active_package_listing: PackageListing, - user: UserType, -): - url = ( - f"/api/cyberstorm/listing/{active_package_listing.community.identifier}/" - f"{active_package_listing.package.namespace.name}/" - f"invalid_package_name/{url_action}/" - ) - - perform_404_test( - api_client=api_client, - active_package_listing=active_package_listing, - user=user, - url=url, - ) - - @pytest.mark.django_db @patch( "thunderstore.community.models.package_listing.PackageListing.can_user_manage_approval_status" @@ -219,3 +194,45 @@ def test_reject_package_listing_permission_error( response = api_client.post(url, data=data, content_type="application/json") assert response.status_code == 403 + + +@pytest.mark.django_db +@pytest.mark.parametrize("user_type", TestUserTypes.options()) +def test_report_package_listing( + active_package_listing: PackageListing, + api_client: APIClient, + user_type: str, +): + expected_status_code_map = { + TestUserTypes.no_user: 401, + TestUserTypes.unauthenticated: 401, + TestUserTypes.regular_user: 200, + TestUserTypes.deactivated_user: 403, + TestUserTypes.service_account: 403, + TestUserTypes.site_admin: 200, + TestUserTypes.superuser: 200, + } + + perform_package_listing_action_test( + api_client=api_client, + package_listing=active_package_listing, + user_type=user_type, + url=get_report_url(active_package_listing), + data={"reason": "Spam"}, + expected_status_code_map=expected_status_code_map, + ) + + +@pytest.mark.django_db +def test_report_package_listing_required_fields( + api_client: APIClient, + active_package_listing: PackageListing, +): + user = TestUserTypes.get_user_by_type(TestUserTypes.site_admin) + api_client.force_authenticate(user=user) + + url = get_report_url(active_package_listing) + response = api_client.post(url, data={}, content_type="application/json") + + assert response.status_code == 400 + assert response.json() == {"reason": ["This field is required."]} diff --git a/django/thunderstore/api/urls.py b/django/thunderstore/api/urls.py index 229cfca2b..a9db4bed4 100644 --- a/django/thunderstore/api/urls.py +++ b/django/thunderstore/api/urls.py @@ -24,6 +24,7 @@ PackageVersionReadmeAPIView, RatePackageAPIView, RejectPackageListingAPIView, + ReportPackageListingAPIView, TeamAPIView, TeamCreateAPIView, TeamMemberAddAPIView, @@ -85,6 +86,11 @@ ApprovePackageListingAPIView.as_view(), name="cyberstorm.listing.approve", ), + path( + "listing////report/", + ReportPackageListingAPIView.as_view(), + name="cyberstorm.listing.report", + ), path( "listing////reject/", RejectPackageListingAPIView.as_view(), From 01349c9d5656ecbfc32ac4eb14dd247b563d543b Mon Sep 17 00:00:00 2001 From: Roffenlund Date: Thu, 4 Sep 2025 13:28:11 +0300 Subject: [PATCH 36/61] Implement service for unlisting package listing Refs. TS-2597 --- .../cyberstorm/services/package_listing.py | 11 ++++++++ .../services/test_package_listing_services.py | 27 +++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/django/thunderstore/api/cyberstorm/services/package_listing.py b/django/thunderstore/api/cyberstorm/services/package_listing.py index 13fc2579b..3e78ef689 100644 --- a/django/thunderstore/api/cyberstorm/services/package_listing.py +++ b/django/thunderstore/api/cyberstorm/services/package_listing.py @@ -1,5 +1,6 @@ from django.db import transaction +from thunderstore.core.exceptions import PermissionValidationError from thunderstore.core.types import UserType from thunderstore.permissions.utils import validate_user from thunderstore.repository.models import Package, PackageListing, PackageVersion @@ -75,3 +76,13 @@ def report_package_listing( package_version=package_version, description=description, ) + + +@transaction.atomic +def unlist_package_listing(agent: UserType, listing: PackageListing) -> None: + + user = validate_user(agent) + if not user.is_superuser: + raise PermissionValidationError("Only superusers can unlist packages") + + listing.package.deactivate() diff --git a/django/thunderstore/api/cyberstorm/tests/services/test_package_listing_services.py b/django/thunderstore/api/cyberstorm/tests/services/test_package_listing_services.py index 1e2520dc6..2bced643b 100644 --- a/django/thunderstore/api/cyberstorm/tests/services/test_package_listing_services.py +++ b/django/thunderstore/api/cyberstorm/tests/services/test_package_listing_services.py @@ -4,6 +4,7 @@ from thunderstore.api.cyberstorm.services.package_listing import ( approve_package_listing, reject_package_listing, + unlist_package_listing, update_categories, ) from thunderstore.community.consts import PackageListingReviewStatus @@ -145,3 +146,29 @@ def test_reject_package_listing_no_community_membership(active_package_listing, notes="This package contains inappropriate content.", listing=active_package_listing, ) + + +@pytest.mark.django_db +@pytest.mark.parametrize( + "user_role, can_unlist", + [ + (TestUserTypes.no_user, False), + (TestUserTypes.unauthenticated, False), + (TestUserTypes.regular_user, False), + (TestUserTypes.deactivated_user, False), + (TestUserTypes.service_account, False), + (TestUserTypes.site_admin, False), + (TestUserTypes.superuser, True), + ], +) +def test_unlist_package_listing(active_package_listing, user_role, can_unlist): + agent = TestUserTypes.get_user_by_type(user_role) + + if can_unlist: + unlist_package_listing(agent=agent, listing=active_package_listing) + active_package_listing.refresh_from_db() + assert active_package_listing.package.is_active is False + else: + with pytest.raises(PermissionValidationError): + unlist_package_listing(agent=agent, listing=active_package_listing) + assert active_package_listing.package.is_active is True From 3b42da50c02e0788f6ee0b12838cc6b3fb488b5b Mon Sep 17 00:00:00 2001 From: Roffenlund Date: Thu, 4 Sep 2025 13:29:31 +0300 Subject: [PATCH 37/61] Implement unlist view Implement POST view for unlisting package listing. Refs. TS-2597 --- .../tests/test_package_listing_actions.py | 44 ++++++++++++++++++- .../api/cyberstorm/views/__init__.py | 2 + .../views/package_listing_actions.py | 20 +++++++++ 3 files changed, 64 insertions(+), 2 deletions(-) diff --git a/django/thunderstore/api/cyberstorm/tests/test_package_listing_actions.py b/django/thunderstore/api/cyberstorm/tests/test_package_listing_actions.py index 5d5469ffa..8b1185b63 100644 --- a/django/thunderstore/api/cyberstorm/tests/test_package_listing_actions.py +++ b/django/thunderstore/api/cyberstorm/tests/test_package_listing_actions.py @@ -29,6 +29,18 @@ def get_reject_url(package_listing): def get_report_url(package_listing): return f"{get_base_url(package_listing)}/report/" +def get_unlist_url(package_listing): + return f"{get_base_url(package_listing)}/unlist/" + + +def perform_404_test( + api_client: APIClient, + active_package_listing: PackageListing, + user: UserType, + url: str, +): + active_package_listing.package.owner.add_member(user, role="owner") + api_client.force_authenticate(user=user) def perform_package_listing_action_test( @@ -52,8 +64,8 @@ def perform_package_listing_action_test( package_listing.refresh_from_db() - if not expected_status_code_map: - expected_status_code_map = { + if expected_status_code is None: + expected_status_code = { TestUserTypes.no_user: 401, TestUserTypes.unauthenticated: 401, TestUserTypes.regular_user: 403, @@ -236,3 +248,31 @@ def test_report_package_listing_required_fields( assert response.status_code == 400 assert response.json() == {"reason": ["This field is required."]} + + +@pytest.mark.django_db +@pytest.mark.parametrize("user_type", TestUserTypes.options()) +def test_unlist_package_listing( + api_client: APIClient, + active_package_listing: PackageListing, + user_type: str, +): + + expected_status_code = { + TestUserTypes.no_user: 401, + TestUserTypes.unauthenticated: 401, + TestUserTypes.regular_user: 403, + TestUserTypes.deactivated_user: 403, + TestUserTypes.service_account: 403, + TestUserTypes.site_admin: 403, + TestUserTypes.superuser: 200, + } + + perform_package_listing_action_test( + api_client=api_client, + package_listing=active_package_listing, + user_type=user_type, + url=get_unlist_url(active_package_listing), + data={}, + expected_status_code=expected_status_code, + ) diff --git a/django/thunderstore/api/cyberstorm/views/__init__.py b/django/thunderstore/api/cyberstorm/views/__init__.py index 1988749b7..d79a3ff94 100644 --- a/django/thunderstore/api/cyberstorm/views/__init__.py +++ b/django/thunderstore/api/cyberstorm/views/__init__.py @@ -8,6 +8,7 @@ ApprovePackageListingAPIView, RejectPackageListingAPIView, ReportPackageListingAPIView, + UnlistPackageListingAPIView, UpdatePackageListingCategoriesAPIView, ) from .package_listing_list import ( @@ -69,4 +70,5 @@ "ApprovePackageListingAPIView", "UpdateTeamAPIView", "ReportPackageListingAPIView", + "UnlistPackageListingAPIView", ] diff --git a/django/thunderstore/api/cyberstorm/views/package_listing_actions.py b/django/thunderstore/api/cyberstorm/views/package_listing_actions.py index 1a439c171..8cfa6ac17 100644 --- a/django/thunderstore/api/cyberstorm/views/package_listing_actions.py +++ b/django/thunderstore/api/cyberstorm/views/package_listing_actions.py @@ -14,6 +14,7 @@ approve_package_listing, reject_package_listing, report_package_listing, + unlist_package_listing, update_categories, ) from thunderstore.api.utils import conditional_swagger_auto_schema @@ -158,3 +159,22 @@ def post(self, request, *args, **kwargs) -> Response: ) return Response({"message": "Success"}, status=status.HTTP_200_OK) + + +class UnlistPackageListingAPIView(APIView): + permission_classes = [IsAuthenticated] + + @swagger_auto_schema( + operation_id="cyberstorm.package_listing.unlist", + request_body=None, + responses={200: "Success"}, + tags=["cyberstorm"], + ) + def post(self, request, *args, **kwargs) -> Response: + listing = get_package_listing( + namespace_id=kwargs["namespace_id"], + package_name=kwargs["package_name"], + community_id=kwargs["community_id"], + ) + unlist_package_listing(agent=request.user, listing=listing) + return Response({"message": "Success"}, status=status.HTTP_200_OK) From 1974378f7473c0993a71b1bf0d6a0dfe8901d2b6 Mon Sep 17 00:00:00 2001 From: Roffenlund Date: Thu, 4 Sep 2025 13:30:09 +0300 Subject: [PATCH 38/61] Implement URL for unlisting package listing Refs. TS-2597 --- django/thunderstore/api/urls.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/django/thunderstore/api/urls.py b/django/thunderstore/api/urls.py index a9db4bed4..3ff847052 100644 --- a/django/thunderstore/api/urls.py +++ b/django/thunderstore/api/urls.py @@ -31,6 +31,7 @@ TeamMemberListAPIView, TeamMemberRemoveAPIView, TeamServiceAccountListAPIView, + UnlistPackageListingAPIView, UpdatePackageListingCategoriesAPIView, UpdateTeamAPIView, ) @@ -96,6 +97,11 @@ RejectPackageListingAPIView.as_view(), name="cyberstorm.listing.reject", ), + path( + "listing////unlist/", + UnlistPackageListingAPIView.as_view(), + name="cyberstorm.listing.unlist", + ), path( "package///latest/changelog/", PackageVersionChangelogAPIView.as_view(), From 40f44addb07675925484a9a5bd54f1a6588a2fa6 Mon Sep 17 00:00:00 2001 From: Roffenlund Date: Tue, 23 Sep 2025 15:35:50 +0300 Subject: [PATCH 39/61] Add endpoint to test endpoints tests Add the endpoint to test_endpoints.py. Remove the check for empty request schemas as some POST endpoints might have empty request payloads. Refs. TS-2597 --- django/thunderstore/api/cyberstorm/tests/endpoint_data.py | 1 + django/thunderstore/api/cyberstorm/tests/utils.py | 3 --- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/django/thunderstore/api/cyberstorm/tests/endpoint_data.py b/django/thunderstore/api/cyberstorm/tests/endpoint_data.py index 473e32751..69834e091 100644 --- a/django/thunderstore/api/cyberstorm/tests/endpoint_data.py +++ b/django/thunderstore/api/cyberstorm/tests/endpoint_data.py @@ -33,6 +33,7 @@ "/api/cyberstorm/listing/{community_id}/{namespace_id}/{package_name}/report/": { "reason": "Spam", }, + "/api/cyberstorm/listing/{community_id}/{namespace_id}/{package_name}/unlist/": {}, "/api/cyberstorm/package/{namespace_id}/{package_name}/deprecate/": { "deprecate": True, }, diff --git a/django/thunderstore/api/cyberstorm/tests/utils.py b/django/thunderstore/api/cyberstorm/tests/utils.py index f30f9b0d7..6a5b0095e 100644 --- a/django/thunderstore/api/cyberstorm/tests/utils.py +++ b/django/thunderstore/api/cyberstorm/tests/utils.py @@ -206,9 +206,6 @@ def validate_request_body_against_schema( errors = [] req_schema = get_request_body_schema(schema, path, method) - if not req_schema: - errors.append(f"No request body schema found for {path}") - return errors try: validate(instance=request_body, schema=req_schema, resolver=resolver) From 09e29c42e12516eacb9f22085cbec184015d4fa4 Mon Sep 17 00:00:00 2001 From: Daniel Vainio Date: Wed, 1 Oct 2025 11:39:24 +0300 Subject: [PATCH 40/61] Sort endpoints in alphabetical order Sort endpoints in endpoint_data.py in alphabetical order. Refs. TS-2597 --- .../api/cyberstorm/tests/endpoint_data.py | 26 +++++++++---------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/django/thunderstore/api/cyberstorm/tests/endpoint_data.py b/django/thunderstore/api/cyberstorm/tests/endpoint_data.py index 69834e091..de2f2abbb 100644 --- a/django/thunderstore/api/cyberstorm/tests/endpoint_data.py +++ b/django/thunderstore/api/cyberstorm/tests/endpoint_data.py @@ -6,15 +6,15 @@ "/api/cyberstorm/listing/{community_id}/", "/api/cyberstorm/listing/{community_id}/{namespace_id}/", "/api/cyberstorm/listing/{community_id}/{namespace_id}/{package_name}/", - "/api/cyberstorm/listing/{community_id}/{namespace_id}/{package_name}/status/", "/api/cyberstorm/listing/{community_id}/{namespace_id}/{package_name}/dependants/", + "/api/cyberstorm/listing/{community_id}/{namespace_id}/{package_name}/status/", "/api/cyberstorm/package/{community_id}/{namespace_id}/{package_name}/permissions/", "/api/cyberstorm/package/{namespace_id}/{package_name}/latest/changelog/", "/api/cyberstorm/package/{namespace_id}/{package_name}/latest/readme/", "/api/cyberstorm/package/{namespace_id}/{package_name}/v/{version_number}/", "/api/cyberstorm/package/{namespace_id}/{package_name}/v/{version_number}/changelog/", - "/api/cyberstorm/package/{namespace_id}/{package_name}/v/{version_number}/readme/", "/api/cyberstorm/package/{namespace_id}/{package_name}/v/{version_number}/dependencies/", + "/api/cyberstorm/package/{namespace_id}/{package_name}/v/{version_number}/readme/", "/api/cyberstorm/package/{namespace_id}/{package_name}/versions/", "/api/cyberstorm/team/{team_id}/", "/api/cyberstorm/team/{team_id}/member/", @@ -22,38 +22,36 @@ ], "POST": { "/api/cyberstorm/listing/{community_id}/{namespace_id}/{package_name}/approve/": { - "internal_notes": "This is an example internal note", + "internal_notes": "This is an example internal note" }, "/api/cyberstorm/listing/{community_id}/{namespace_id}/{package_name}/reject/": { - "rejection_reason": "This is an example rejection reason", - }, - "/api/cyberstorm/listing/{community_id}/{namespace_id}/{package_name}/update/": { - "categories": ["test"], + "rejection_reason": "This is an example rejection reason" }, "/api/cyberstorm/listing/{community_id}/{namespace_id}/{package_name}/report/": { "reason": "Spam", }, "/api/cyberstorm/listing/{community_id}/{namespace_id}/{package_name}/unlist/": {}, + "/api/cyberstorm/listing/{community_id}/{namespace_id}/{package_name}/update/": { + "categories": ["test"] + }, "/api/cyberstorm/package/{namespace_id}/{package_name}/deprecate/": { - "deprecate": True, + "deprecate": True }, "/api/cyberstorm/package/{namespace_id}/{package_name}/rate/": { - "target_state": "rated", - }, - "/api/cyberstorm/team/create/": { - "name": "TestTeam", + "target_state": "rated" }, + "/api/cyberstorm/team/create/": {"name": "TestTeam"}, "/api/cyberstorm/team/{team_name}/member/add/": { "username": "TestUser", "role": "member", }, "/api/cyberstorm/team/{team_name}/service-account/create/": { - "nickname": "TestServiceAccount", + "nickname": "TestServiceAccount" }, }, "PATCH": { "/api/cyberstorm/team/{team_name}/update/": { - "donation_link": "https://example.com/donate", + "donation_link": "https://example.com/donate" }, }, "DELETE": [ From a84219cbbe3e84000b896cac70879e0eb4bd4bb8 Mon Sep 17 00:00:00 2001 From: Daniel Vainio Date: Wed, 1 Oct 2025 11:40:40 +0300 Subject: [PATCH 41/61] Update parameter type in test Use empty dict instead of None as default value in test_package_listing_action test. Refs. TS-2597 --- .../api/cyberstorm/tests/test_package_listing_actions.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/django/thunderstore/api/cyberstorm/tests/test_package_listing_actions.py b/django/thunderstore/api/cyberstorm/tests/test_package_listing_actions.py index 8b1185b63..8ab96ce84 100644 --- a/django/thunderstore/api/cyberstorm/tests/test_package_listing_actions.py +++ b/django/thunderstore/api/cyberstorm/tests/test_package_listing_actions.py @@ -1,4 +1,5 @@ import json +from typing import Optional from unittest.mock import patch import pytest @@ -49,7 +50,7 @@ def perform_package_listing_action_test( user_type: str, url: str, data: dict, - expected_status_code_map: dict = None, + expected_status_code: Optional[dict] = None, ): user = TestUserTypes.get_user_by_type(user_type) From 4f3b42d251e3bced9e03407c703cb5b6bc6ed5d9 Mon Sep 17 00:00:00 2001 From: Daniel Vainio Date: Wed, 1 Oct 2025 11:42:15 +0300 Subject: [PATCH 42/61] Use conditional_swagger_auto_schema Use conditional_swagger_auto_schema in order to hide endpoint depending on settings. Refs. TS-2597 --- .../api/cyberstorm/views/package_listing_actions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/django/thunderstore/api/cyberstorm/views/package_listing_actions.py b/django/thunderstore/api/cyberstorm/views/package_listing_actions.py index 8cfa6ac17..5ff410ea6 100644 --- a/django/thunderstore/api/cyberstorm/views/package_listing_actions.py +++ b/django/thunderstore/api/cyberstorm/views/package_listing_actions.py @@ -164,7 +164,7 @@ def post(self, request, *args, **kwargs) -> Response: class UnlistPackageListingAPIView(APIView): permission_classes = [IsAuthenticated] - @swagger_auto_schema( + @conditional_swagger_auto_schema( operation_id="cyberstorm.package_listing.unlist", request_body=None, responses={200: "Success"}, From d5eb376c77d138b07d38de5dcfa923f315f11a76 Mon Sep 17 00:00:00 2001 From: Daniel Vainio Date: Thu, 2 Oct 2025 15:26:48 +0300 Subject: [PATCH 43/61] Fix variable name in tests expected_status_code -> expected_status_code_map. Refs. TS-2597 --- .../cyberstorm/tests/test_package_listing_actions.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/django/thunderstore/api/cyberstorm/tests/test_package_listing_actions.py b/django/thunderstore/api/cyberstorm/tests/test_package_listing_actions.py index 8ab96ce84..726ca81c8 100644 --- a/django/thunderstore/api/cyberstorm/tests/test_package_listing_actions.py +++ b/django/thunderstore/api/cyberstorm/tests/test_package_listing_actions.py @@ -30,6 +30,8 @@ def get_reject_url(package_listing): def get_report_url(package_listing): return f"{get_base_url(package_listing)}/report/" + + def get_unlist_url(package_listing): return f"{get_base_url(package_listing)}/unlist/" @@ -50,7 +52,7 @@ def perform_package_listing_action_test( user_type: str, url: str, data: dict, - expected_status_code: Optional[dict] = None, + expected_status_code_map: Optional[dict] = None, ): user = TestUserTypes.get_user_by_type(user_type) @@ -65,8 +67,8 @@ def perform_package_listing_action_test( package_listing.refresh_from_db() - if expected_status_code is None: - expected_status_code = { + if expected_status_code_map is None: + expected_status_code_map = { TestUserTypes.no_user: 401, TestUserTypes.unauthenticated: 401, TestUserTypes.regular_user: 403, @@ -259,7 +261,7 @@ def test_unlist_package_listing( user_type: str, ): - expected_status_code = { + expected_status_code_map = { TestUserTypes.no_user: 401, TestUserTypes.unauthenticated: 401, TestUserTypes.regular_user: 403, @@ -275,5 +277,5 @@ def test_unlist_package_listing( user_type=user_type, url=get_unlist_url(active_package_listing), data={}, - expected_status_code=expected_status_code, + expected_status_code_map=expected_status_code_map, ) From a51c2c16dcd201ce4089e6073a0bf7b3882866f3 Mon Sep 17 00:00:00 2001 From: Daniel Vainio Date: Fri, 3 Oct 2025 16:40:48 +0300 Subject: [PATCH 44/61] Optimize dependencies list queryset query Annotate the queryset with active_package_versions instead of calling PackageVersion.is_removed, which avoids triggering an N+1 query. Refs. TS-2672 --- .../api/cyberstorm/serializers/package.py | 10 +++++- .../tests/test_package_version_list.py | 34 +++++++++++++++++-- .../api/cyberstorm/tests/test_query_count.py | 2 +- .../cyberstorm/views/package_version_list.py | 9 ++++- 4 files changed, 50 insertions(+), 5 deletions(-) diff --git a/django/thunderstore/api/cyberstorm/serializers/package.py b/django/thunderstore/api/cyberstorm/serializers/package.py index 5ed8ace4e..126509c03 100644 --- a/django/thunderstore/api/cyberstorm/serializers/package.py +++ b/django/thunderstore/api/cyberstorm/serializers/package.py @@ -67,7 +67,15 @@ class CyberstormPackageDependencySerializer(serializers.Serializer): name = serializers.CharField() namespace = serializers.CharField(source="package.namespace.name") version_number = serializers.CharField() - is_removed = serializers.BooleanField() + is_removed = serializers.SerializerMethodField() + + def get_is_removed(self, obj: PackageVersion) -> bool: + package_is_removed = not ( + obj.package.is_active and obj.package_has_active_versions + ) + if package_is_removed: + return True + return not obj.is_active def get_description(self, obj: PackageVersion) -> str: return ( diff --git a/django/thunderstore/api/cyberstorm/tests/test_package_version_list.py b/django/thunderstore/api/cyberstorm/tests/test_package_version_list.py index 2ccbae4e2..ef13b7ede 100644 --- a/django/thunderstore/api/cyberstorm/tests/test_package_version_list.py +++ b/django/thunderstore/api/cyberstorm/tests/test_package_version_list.py @@ -1,8 +1,6 @@ from typing import Optional import pytest -from django.db import connection -from django.test.utils import CaptureQueriesContext from rest_framework.test import APIClient from thunderstore.repository.factories import PackageFactory, PackageVersionFactory @@ -134,3 +132,35 @@ def test_package_version_dependencies_list_response(api_client: APIClient) -> No assert response.status_code == 200 assert response.json() == expected_data + + +@pytest.mark.django_db +@pytest.mark.parametrize( + "is_active, is_removed", + [ + (False, True), + (True, False), + ], +) +def test_package_version_dependencies_list_version_is_active( + api_client: APIClient, + is_active: bool, + is_removed: bool, +) -> None: + """Test the is_removed field in the PackageVersionDependenciesListAPIView.""" + + package = PackageFactory(name="TestPackage") + package_version = PackageVersionFactory(package=package) + + dependency = PackageVersionFactory(is_active=is_active) + package_version.dependencies.set([dependency.id]) + + namespace = package.namespace.name + package_name = package.name + + url = f"/api/cyberstorm/package/{namespace}/{package_name}/v/latest/dependencies/" + response = api_client.get(url) + + assert response.status_code == 200 + assert response.json()["results"][0]["is_removed"] == is_removed + assert response.json()["results"][0]["is_active"] == is_active diff --git a/django/thunderstore/api/cyberstorm/tests/test_query_count.py b/django/thunderstore/api/cyberstorm/tests/test_query_count.py index 49960e59c..93b3f10b2 100644 --- a/django/thunderstore/api/cyberstorm/tests/test_query_count.py +++ b/django/thunderstore/api/cyberstorm/tests/test_query_count.py @@ -233,5 +233,5 @@ def test_package_version_dependencies_query_count(api_client: APIClient) -> None client=api_client, method="get", path=path, - max_queries=25, # TODO: this could be optimized further + max_queries=MAX_QUERIES, ) diff --git a/django/thunderstore/api/cyberstorm/views/package_version_list.py b/django/thunderstore/api/cyberstorm/views/package_version_list.py index 927e06fbd..fbd0eb389 100644 --- a/django/thunderstore/api/cyberstorm/views/package_version_list.py +++ b/django/thunderstore/api/cyberstorm/views/package_version_list.py @@ -1,4 +1,4 @@ -from django.db.models import QuerySet +from django.db.models import Exists, OuterRef, QuerySet from rest_framework import serializers from rest_framework.generics import ListAPIView, get_object_or_404 @@ -54,6 +54,13 @@ def get_queryset(self) -> QuerySet[PackageVersion]: qs = ( package_version.dependencies.all() .select_related("package", "package__namespace") + .annotate( + package_has_active_versions=Exists( + PackageVersion.objects.filter( + package_id=OuterRef("package__pk"), is_active=True + ) + ) + ) .order_by("package__namespace__name", "package__name") ) From 3cd010900dbbc2fb80669d77899e4e46085436eb Mon Sep 17 00:00:00 2001 From: Daniel Vainio Date: Mon, 6 Oct 2025 10:10:02 +0300 Subject: [PATCH 45/61] Remove redundant test function Remove function which is not being used by any tests. Refs. TS-2597 --- .../cyberstorm/tests/test_package_listing_actions.py | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/django/thunderstore/api/cyberstorm/tests/test_package_listing_actions.py b/django/thunderstore/api/cyberstorm/tests/test_package_listing_actions.py index 726ca81c8..8750b0bb0 100644 --- a/django/thunderstore/api/cyberstorm/tests/test_package_listing_actions.py +++ b/django/thunderstore/api/cyberstorm/tests/test_package_listing_actions.py @@ -36,16 +36,6 @@ def get_unlist_url(package_listing): return f"{get_base_url(package_listing)}/unlist/" -def perform_404_test( - api_client: APIClient, - active_package_listing: PackageListing, - user: UserType, - url: str, -): - active_package_listing.package.owner.add_member(user, role="owner") - api_client.force_authenticate(user=user) - - def perform_package_listing_action_test( api_client: APIClient, package_listing: PackageListing, From 2a4b8b831cf33678197d7711af4a3f56cca6b0f7 Mon Sep 17 00:00:00 2001 From: Daniel Vainio Date: Mon, 6 Oct 2025 10:13:29 +0300 Subject: [PATCH 46/61] Add the unlist action to tests Add the /unlist endpoint to test. Refs. TS-2597 --- .../api/cyberstorm/tests/test_package_listing_actions.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/django/thunderstore/api/cyberstorm/tests/test_package_listing_actions.py b/django/thunderstore/api/cyberstorm/tests/test_package_listing_actions.py index 8750b0bb0..2a6bd790c 100644 --- a/django/thunderstore/api/cyberstorm/tests/test_package_listing_actions.py +++ b/django/thunderstore/api/cyberstorm/tests/test_package_listing_actions.py @@ -8,6 +8,8 @@ from conftest import TestUserTypes, UserType from thunderstore.community.models import PackageCategory, PackageListing +PACKAGE_LISTING_ACTIONS = ["update", "approve", "reject", "report", "unlist"] + def get_base_url(package_listing): namespace_id = package_listing.package.namespace.name @@ -72,7 +74,7 @@ def perform_package_listing_action_test( @pytest.mark.django_db -@pytest.mark.parametrize("url_action", ["update", "approve", "reject", "report"]) +@pytest.mark.parametrize("url_action", PACKAGE_LISTING_ACTIONS) @pytest.mark.parametrize( "invalid_field", ["community_id", "namespace_id", "package_name"] ) From 271aa1a571965347ca2f0c794a1c85ad86d6e467 Mon Sep 17 00:00:00 2001 From: Roffenlund Date: Wed, 24 Sep 2025 16:53:05 +0300 Subject: [PATCH 47/61] Implement service for updating team member Implement a service in the service layer in the cyberstorm API for updating team members. Refs. TS-2316 --- .../api/cyberstorm/services/team.py | 22 ++++- .../tests/services/test_team_services.py | 80 +++++++++++++++++++ 2 files changed, 100 insertions(+), 2 deletions(-) diff --git a/django/thunderstore/api/cyberstorm/services/team.py b/django/thunderstore/api/cyberstorm/services/team.py index 02e79577b..45d282f50 100644 --- a/django/thunderstore/api/cyberstorm/services/team.py +++ b/django/thunderstore/api/cyberstorm/services/team.py @@ -5,8 +5,8 @@ from thunderstore.account.models import ServiceAccount from thunderstore.core.exceptions import PermissionValidationError from thunderstore.core.types import UserType -from thunderstore.repository.models import Namespace, Team -from thunderstore.repository.models.team import TeamMember, TeamMemberRole +from thunderstore.repository.models import Namespace, Team, TeamMember +from thunderstore.repository.models.team import TeamMemberRole @transaction.atomic @@ -73,3 +73,21 @@ def delete_service_account(agent: UserType, service_account: ServiceAccount): team.ensure_user_can_access(agent) team.ensure_can_delete_service_account(agent) return service_account.delete() + + +@transaction.atomic +def update_team_member( + agent: UserType, + team_member: TeamMember, + role: str, +) -> TeamMember: + team = team_member.team + + team.ensure_user_can_access(agent) + team.ensure_user_can_manage_members(agent) + team.ensure_member_role_can_be_changed(team_member, role) + + team_member.role = role + team_member.save() + + return team_member diff --git a/django/thunderstore/api/cyberstorm/tests/services/test_team_services.py b/django/thunderstore/api/cyberstorm/tests/services/test_team_services.py index 7faf2fa77..8e10442b2 100644 --- a/django/thunderstore/api/cyberstorm/tests/services/test_team_services.py +++ b/django/thunderstore/api/cyberstorm/tests/services/test_team_services.py @@ -250,3 +250,83 @@ def test_delete_service_account_not_owner(service_account): error_msg = "Must be an owner to delete a service account" with pytest.raises(PermissionValidationError, match=error_msg): team_services.delete_service_account(user, service_account) + + +@pytest.mark.django_db +@pytest.mark.parametrize("user_type", TestUserTypes.options()) +def test_update_team_member_user_roles(user_type, team_member): + user = TestUserTypes.get_user_by_type(user_type) + team = team_member.team + + if user_type not in TestUserTypes.fake_users(): + team.add_member(user=user, role=TeamMemberRole.owner) + + expected_result_mapping = { + TestUserTypes.no_user: (False, PermissionValidationError), + TestUserTypes.unauthenticated: (False, PermissionValidationError), + TestUserTypes.regular_user: (True, None), + TestUserTypes.deactivated_user: (False, PermissionValidationError), + TestUserTypes.service_account: (False, PermissionValidationError), + TestUserTypes.site_admin: (True, None), + TestUserTypes.superuser: (True, None), + } + + should_raise = expected_result_mapping[user_type][0] is False + + if should_raise: + with pytest.raises(expected_result_mapping[user_type][1]): + team_services.update_team_member(user, team_member, TeamMemberRole.owner) + else: + update_user = team_services.update_team_member( + user, team_member, TeamMemberRole.owner + ) + assert update_user.role == TeamMemberRole.owner + + +@pytest.mark.django_db +def test_update_team_member_user_cannot_access_team(user, team_member): + error_msg = "Must be a member to access team" + with pytest.raises(PermissionValidationError, match=error_msg): + team_services.update_team_member(user, team_member, TeamMemberRole.owner) + + +@pytest.mark.django_db +def test_update_team_member_user_cannot_manage_members(team_member, user): + team = team_member.team + team.add_member(user=user, role=TeamMemberRole.member) + + error_msg = "Must be an owner to manage team members" + with pytest.raises(PermissionValidationError, match=error_msg): + team_services.update_team_member(user, team_member, TeamMemberRole.owner) + + +@pytest.mark.django_db +def test_update_team_member_invalid_role(team_member, user): + team = team_member.team + team.add_member(user=user, role=TeamMemberRole.owner) + + with pytest.raises(ValidationError, match="New role is invalid"): + team_services.update_team_member(user, team_member, "invalid_role") + + +@pytest.mark.django_db +def test_update_team_member_success(team_member, user): + team = team_member.team + team.add_member(user=user, role=TeamMemberRole.owner) + + assert team_member.role == TeamMemberRole.member + + updated_member = team_services.update_team_member( + user, team_member, TeamMemberRole.owner + ) + + assert updated_member.role == TeamMemberRole.owner + + +@pytest.mark.django_db +def test_update_team_member_cannot_remove_last_owner(team_owner): + error_msg = "Cannot remove last owner from team" + with pytest.raises(ValidationError, match=error_msg): + team_services.update_team_member( + team_owner.user, team_owner, TeamMemberRole.member + ) From 0891dd3d2dfc789efb3ec5f09456fdd3e55c35da Mon Sep 17 00:00:00 2001 From: Roffenlund Date: Wed, 24 Sep 2025 16:54:04 +0300 Subject: [PATCH 48/61] Implement serializer for updating team members Refs. TS-2316 --- django/thunderstore/api/cyberstorm/serializers/__init__.py | 2 ++ django/thunderstore/api/cyberstorm/serializers/team.py | 7 +++++++ 2 files changed, 9 insertions(+) diff --git a/django/thunderstore/api/cyberstorm/serializers/__init__.py b/django/thunderstore/api/cyberstorm/serializers/__init__.py index 344cf49da..c342b6922 100644 --- a/django/thunderstore/api/cyberstorm/serializers/__init__.py +++ b/django/thunderstore/api/cyberstorm/serializers/__init__.py @@ -18,6 +18,7 @@ CyberstormTeamAddMemberRequestSerializer, CyberstormTeamAddMemberResponseSerializer, CyberstormTeamMemberSerializer, + CyberstormTeamMemberUpdateSerializer, CyberstormTeamSerializer, CyberstormTeamUpdateSerializer, ) @@ -36,6 +37,7 @@ "CyberstormPackageTeamSerializer", "CyberstormServiceAccountSerializer", "CyberstormTeamMemberSerializer", + "CyberstormTeamMemberUpdateSerializer", "CyberstormTeamSerializer", "PackagePermissionsSerializer", "CyberstormTeamUpdateSerializer", diff --git a/django/thunderstore/api/cyberstorm/serializers/team.py b/django/thunderstore/api/cyberstorm/serializers/team.py index f65d4215b..8fd7f71ac 100644 --- a/django/thunderstore/api/cyberstorm/serializers/team.py +++ b/django/thunderstore/api/cyberstorm/serializers/team.py @@ -4,6 +4,7 @@ from rest_framework import serializers from thunderstore.repository.forms import AddTeamMemberForm +from thunderstore.repository.models.team import TeamMemberRole from thunderstore.repository.validators import PackageReferenceComponentValidator from thunderstore.social.utils import get_user_avatar_url @@ -66,3 +67,9 @@ class CyberstormCreateServiceAccountSerializer(serializers.Serializer): nickname = serializers.CharField(max_length=32) team_name = serializers.CharField(read_only=True) api_token = serializers.CharField(read_only=True) + + +class CyberstormTeamMemberUpdateSerializer(serializers.Serializer): + role = serializers.ChoiceField(choices=TeamMemberRole.as_choices()) + team_name = serializers.CharField(source="team.name", read_only=True) + username = serializers.CharField(source="user.username", read_only=True) From 826b5455af12b1c089377e53e03ec650bce4abf4 Mon Sep 17 00:00:00 2001 From: Roffenlund Date: Wed, 24 Sep 2025 16:54:54 +0300 Subject: [PATCH 49/61] Implement view for updating team member Implement an APIView in the cyberstorm API responsible for updating team members. The view utilizes the service layer functionality for permission checks and performing the update action to the database object. Refs. TS-2316 --- .../api/cyberstorm/views/__init__.py | 2 ++ .../thunderstore/api/cyberstorm/views/team.py | 32 +++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/django/thunderstore/api/cyberstorm/views/__init__.py b/django/thunderstore/api/cyberstorm/views/__init__.py index d79a3ff94..605eb8986 100644 --- a/django/thunderstore/api/cyberstorm/views/__init__.py +++ b/django/thunderstore/api/cyberstorm/views/__init__.py @@ -34,6 +34,7 @@ TeamMemberRemoveAPIView, TeamServiceAccountListAPIView, UpdateTeamAPIView, + UpdateTeamMemberAPIView, ) from .user import DeleteUserAPIView, DisconnectUserLinkedAccountAPIView @@ -71,4 +72,5 @@ "UpdateTeamAPIView", "ReportPackageListingAPIView", "UnlistPackageListingAPIView", + "UpdateTeamMemberAPIView", ] diff --git a/django/thunderstore/api/cyberstorm/views/team.py b/django/thunderstore/api/cyberstorm/views/team.py index 399f0e9ac..f3c8ae0dd 100644 --- a/django/thunderstore/api/cyberstorm/views/team.py +++ b/django/thunderstore/api/cyberstorm/views/team.py @@ -15,6 +15,7 @@ CyberstormTeamAddMemberRequestSerializer, CyberstormTeamAddMemberResponseSerializer, CyberstormTeamMemberSerializer, + CyberstormTeamMemberUpdateSerializer, CyberstormTeamSerializer, CyberstormTeamUpdateSerializer, ) @@ -25,6 +26,7 @@ disband_team, remove_team_member, update_team, + update_team_member, ) from thunderstore.api.ordering import StrictOrderingFilter from thunderstore.api.utils import ( @@ -239,3 +241,33 @@ def patch(self, request, team_name, *args, **kwargs): return_data = self.serializer_class(instance=updated_team).data return Response(return_data, status=status.HTTP_200_OK) + + +class UpdateTeamMemberAPIView(APIView): + permission_classes = [IsAuthenticated] + serializer_class = CyberstormTeamMemberUpdateSerializer + http_method_names = ["patch"] + + @conditional_swagger_auto_schema( + operation_id="cyberstorm.team.member.update", + tags=["cyberstorm"], + responses={status.HTTP_200_OK: serializer_class}, + ) + def patch(self, request, *args, **kwargs): + team_member = get_object_or_404( + TeamMember.objects.real_users(), + team__name=self.kwargs["team_name"], + user__username=self.kwargs["team_member"], + ) + + serializer = self.serializer_class(data=request.data) + serializer.is_valid(raise_exception=True) + + team_member = update_team_member( + agent=request.user, + team_member=team_member, + role=serializer.validated_data["role"], + ) + + serializer = self.serializer_class(instance=team_member) + return Response(serializer.data, status=status.HTTP_200_OK) From bae84557b30efb70833b67e2f0782b9fff1d3764 Mon Sep 17 00:00:00 2001 From: Roffenlund Date: Wed, 24 Sep 2025 16:56:25 +0300 Subject: [PATCH 50/61] Add URL endpoint for update team member Add a new endpoint for updating team members in the Cyberstorm API. Implement functional tests. Refs. TS-2316 --- .../tests/test_update_team_member.py | 195 ++++++++++++++++++ django/thunderstore/api/urls.py | 6 + 2 files changed, 201 insertions(+) create mode 100644 django/thunderstore/api/cyberstorm/tests/test_update_team_member.py diff --git a/django/thunderstore/api/cyberstorm/tests/test_update_team_member.py b/django/thunderstore/api/cyberstorm/tests/test_update_team_member.py new file mode 100644 index 000000000..9d50d653b --- /dev/null +++ b/django/thunderstore/api/cyberstorm/tests/test_update_team_member.py @@ -0,0 +1,195 @@ +import json + +import pytest +from rest_framework.test import APIClient + +from conftest import TestUserTypes +from thunderstore.core.types import UserType +from thunderstore.repository.factories import TeamFactory, TeamMemberFactory +from thunderstore.repository.models import Team, TeamMember +from thunderstore.repository.models.team import TeamMemberRole + + +def get_update_team_member_url(team_name: str, team_member: str) -> str: + return f"/api/cyberstorm/team/{team_name}/member/{team_member}/update/" + + +def make_request(api_client: APIClient, team_name: str, team_member: str, data: dict): + return api_client.patch( + get_update_team_member_url(team_name, team_member), + json.dumps(data), + content_type="application/json", + ) + + +@pytest.mark.django_db +@pytest.mark.parametrize("user_role", TestUserTypes.options()) +def test_update_team_member_user_roles( + user_role: str, + api_client: APIClient, + team_member: TeamMember, +): + user = TestUserTypes.get_user_by_type(user_role) + team = team_member.team + + if user_role not in TestUserTypes.fake_users(): + team.add_member(user=user, role=TeamMemberRole.owner) + api_client.force_authenticate(user) + + expected_status_code = { + TestUserTypes.no_user: 401, + TestUserTypes.unauthenticated: 401, + TestUserTypes.regular_user: 200, + TestUserTypes.deactivated_user: 403, + TestUserTypes.service_account: 403, + TestUserTypes.site_admin: 200, + TestUserTypes.superuser: 200, + } + + response = make_request( + api_client, + team.name, + team_member.user.username, + {"role": "owner"}, + ) + + assert response.status_code == expected_status_code[user_role] + + if response.status_code == 200: + assert TeamMember.objects.get(pk=team_member.pk).role == TeamMemberRole.owner + + +@pytest.mark.django_db +def test_update_team_member_success( + api_client: APIClient, + user: UserType, + team: Team, +): + TeamMemberFactory(team=team, user=user, role="owner") + api_client.force_authenticate(user) + just_a_member = TeamMemberFactory(team=team, role="owner") + + data = {"role": "member"} + team_member = just_a_member.user.username + response = make_request(api_client, team.name, team_member, data) + response_json = response.json() + + assert response.status_code == 200 + assert response_json["team_name"] == team.name + assert response_json["username"] == just_a_member.user.username + assert response_json["role"] == "member" + assert TeamMember.objects.get(pk=just_a_member.pk).role == "member" + + +@pytest.mark.django_db +def test_update_team_member_fail_user_not_in_team( + api_client: APIClient, + user: UserType, + team: Team, +): + TeamMemberFactory(team=team, user=user, role="owner") + api_client.force_authenticate(user) + another_team = TeamFactory() + member_in_another_team = TeamMemberFactory(team=another_team, role="owner") + + data = {"role": "member"} + team_member = member_in_another_team.user.username + response = make_request(api_client, team.name, team_member, data) + response_json = response.json() + + assert response.status_code == 404 + assert response_json["detail"] == "Not found." + assert TeamMember.objects.get(pk=member_in_another_team.pk).role == "owner" + + +@pytest.mark.django_db +def test_update_team_member_fails_team_does_not_exist( + api_client: APIClient, + user: UserType, +): + api_client.force_authenticate(user) + + data = {"role": "member"} + response = make_request(api_client, "GhostTeam", user.username, data) + response_json = response.json() + + assert response.status_code == 404 + assert response_json["detail"] == "Not found." + + +@pytest.mark.django_db +def test_update_team_member_fails_user_not_authenticated( + api_client: APIClient, + user: UserType, + team: Team, +): + TeamMemberFactory(team=team, user=user, role="owner") + just_a_member = TeamMemberFactory(team=team, role="owner") + + data = {"role": "member"} + team_member = just_a_member.user.username + response = make_request(api_client, team.name, team_member, data) + response_json = response.json() + + assert response.status_code == 401 + assert response_json["detail"] == "Authentication credentials were not provided." + assert TeamMember.objects.get(pk=just_a_member.pk).role == "owner" + + +@pytest.mark.django_db +def test_update_team_member_fails_user_can_not_manage_members( + api_client: APIClient, + user: UserType, + team: Team, +): + TeamMemberFactory(team=team, user=user, role="member") + api_client.force_authenticate(user) + just_a_member = TeamMemberFactory(team=team, role="member") + + data = {"role": "member"} + team_member = just_a_member.user.username + response = make_request(api_client, team.name, team_member, data) + expected_response = { + "non_field_errors": ["Must be an owner to manage team members"] + } + + assert response.status_code == 403 + assert response.json() == expected_response + assert TeamMember.objects.get(pk=just_a_member.pk).role == "member" + + +@pytest.mark.django_db +def test_update_team_member_fails_invalid_role( + api_client: APIClient, + user: UserType, + team: Team, +): + TeamMemberFactory(team=team, user=user, role="owner") + api_client.force_authenticate(user) + just_a_member = TeamMemberFactory(team=team, role="owner") + + data = {"role": "invalid_role"} + team_member = just_a_member.user.username + response = make_request(api_client, team.name, team_member, data) + expected_response = {"role": ['"invalid_role" is not a valid choice.']} + + assert response.status_code == 400 + assert response.json() == expected_response + assert TeamMember.objects.get(pk=just_a_member.pk).role == "owner" + + +@pytest.mark.django_db +def test_update_team_members_fails_last_owner( + api_client: APIClient, + user: UserType, + team: Team, +): + last_owner = TeamMemberFactory(team=team, user=user, role="owner") + api_client.force_authenticate(user) + + data = {"role": "member"} + team_member = last_owner.user.username + response = make_request(api_client, team.name, team_member, data) + + assert response.status_code == 400 + assert TeamMember.objects.get(pk=last_owner.pk).role == "owner" diff --git a/django/thunderstore/api/urls.py b/django/thunderstore/api/urls.py index 3ff847052..ed4bbe6dd 100644 --- a/django/thunderstore/api/urls.py +++ b/django/thunderstore/api/urls.py @@ -34,6 +34,7 @@ UnlistPackageListingAPIView, UpdatePackageListingCategoriesAPIView, UpdateTeamAPIView, + UpdateTeamMemberAPIView, ) cyberstorm_urls = [ @@ -212,4 +213,9 @@ DisconnectUserLinkedAccountAPIView.as_view(), name="cyberstorm.user.linked-account.disconnect", ), + path( + "team//member//update/", + UpdateTeamMemberAPIView.as_view(), + name="cyberstorm.team.member.update", + ), ] From b47ddc644c8859181c8813f97feb3671d0e84e6d Mon Sep 17 00:00:00 2001 From: Roffenlund Date: Wed, 24 Sep 2025 16:57:10 +0300 Subject: [PATCH 51/61] Update form and form view for update team member Update the form which is responsible for updating team members in the legacy application to use the service layer functionality. Update the view and tests accordingly to the new changes. Refs. TS-2316 --- django/thunderstore/repository/forms/team.py | 46 +++++++++++-------- .../repository/tests/test_team_forms.py | 4 ++ .../repository/views/team_settings.py | 10 ++-- 3 files changed, 36 insertions(+), 24 deletions(-) diff --git a/django/thunderstore/repository/forms/team.py b/django/thunderstore/repository/forms/team.py index 15b98d9cf..f55066689 100644 --- a/django/thunderstore/repository/forms/team.py +++ b/django/thunderstore/repository/forms/team.py @@ -4,7 +4,11 @@ from django.contrib.auth import get_user_model from django.core.exceptions import ObjectDoesNotExist, ValidationError -from thunderstore.api.cyberstorm.services.team import remove_team_member, update_team +from thunderstore.api.cyberstorm.services.team import ( + remove_team_member, + update_team, + update_team_member, +) from thunderstore.core.exceptions import PermissionValidationError from thunderstore.core.types import UserType from thunderstore.repository.models import ( @@ -116,31 +120,33 @@ def __init__(self, user: Optional[UserType], *args, **kwargs): super().__init__(*args, **kwargs) self.user = user - def clean_role(self): - new_role = self.cleaned_data.get("role", None) - try: - team = self.instance.team - except ObjectDoesNotExist: - team = None - if team: - team.ensure_member_role_can_be_changed( - member=self.instance, new_role=new_role - ) - else: - raise ValidationError("Team is missing") - return new_role - def clean(self): + if not self.instance.pk: + raise ValidationError("Missing team member instance") + try: - team = self.instance.team + self.instance.team except ObjectDoesNotExist: - team = None - if team: - team.ensure_user_can_manage_members(self.user) - else: raise ValidationError("Team is missing") + return super().clean() + def save(self, *args, **kwargs): + if self.errors: + raise ValidationError(self.errors) + + try: + update_team_member( + agent=self.user, + team_member=self.instance, + role=self.cleaned_data["role"], + ) + except ValidationError as e: + self.add_error(None, e) + raise ValidationError(self.errors) + + return self.instance + class DisbandTeamForm(forms.ModelForm): verification = forms.CharField() diff --git a/django/thunderstore/repository/tests/test_team_forms.py b/django/thunderstore/repository/tests/test_team_forms.py index 944e280ec..3771c0597 100644 --- a/django/thunderstore/repository/tests/test_team_forms.py +++ b/django/thunderstore/repository/tests/test_team_forms.py @@ -385,6 +385,8 @@ def test_form_edit_team_member( membership.refresh_from_db() assert membership.role == new_role else: + with pytest.raises(ValidationError): + form.save() assert form.is_valid() is False assert form.errors @@ -403,6 +405,8 @@ def test_form_edit_team_member_remove_last_owner() -> None: instance=last_owner, data={"role": TeamMemberRole.member}, ) + with pytest.raises(ValidationError): + form.save() assert form.is_valid() is False assert "Cannot remove last owner from team" in str(repr(form.errors)) diff --git a/django/thunderstore/repository/views/team_settings.py b/django/thunderstore/repository/views/team_settings.py index e77bafbde..1dc83c3be 100644 --- a/django/thunderstore/repository/views/team_settings.py +++ b/django/thunderstore/repository/views/team_settings.py @@ -110,14 +110,16 @@ def get_context_data(self, **kwargs): return context def form_invalid(self, form): - messages.error( - self.request, "There was a problem performing the requested action" - ) + error_msg = "There was a problem performing the requested action" + messages.error(self.request, error_msg) capture_exception(ValidationError(form.errors)) return super().form_invalid(form) def form_valid(self, form): - form.save() + try: + form.save() + except ValidationError: + return self.form_invalid(form) messages.success(self.request, "Action performed successfully") return redirect(self.object.settings_url) From 94f5809c97f9557eda60be3bb5af7b42899420ae Mon Sep 17 00:00:00 2001 From: Roffenlund Date: Wed, 24 Sep 2025 17:30:59 +0300 Subject: [PATCH 52/61] Add endpoint to endpoint tests Add endpoint to endpoint tests, update view swagger docs with request body. Add username to get_parameter_values util. Refs. TS-2316 --- django/thunderstore/api/cyberstorm/tests/endpoint_data.py | 3 +++ .../thunderstore/api/cyberstorm/tests/test_endpoints.py | 8 +++++++- django/thunderstore/api/cyberstorm/tests/utils.py | 1 + django/thunderstore/api/cyberstorm/views/team.py | 1 + 4 files changed, 12 insertions(+), 1 deletion(-) diff --git a/django/thunderstore/api/cyberstorm/tests/endpoint_data.py b/django/thunderstore/api/cyberstorm/tests/endpoint_data.py index de2f2abbb..ef81ed75c 100644 --- a/django/thunderstore/api/cyberstorm/tests/endpoint_data.py +++ b/django/thunderstore/api/cyberstorm/tests/endpoint_data.py @@ -53,6 +53,9 @@ "/api/cyberstorm/team/{team_name}/update/": { "donation_link": "https://example.com/donate" }, + "/api/cyberstorm/team/{team_name}/member/{team_member}/update/": { + "role": "owner", + }, }, "DELETE": [ "/api/cyberstorm/team/{team_name}/disband/", diff --git a/django/thunderstore/api/cyberstorm/tests/test_endpoints.py b/django/thunderstore/api/cyberstorm/tests/test_endpoints.py index 1e5cf698e..56fe05545 100644 --- a/django/thunderstore/api/cyberstorm/tests/test_endpoints.py +++ b/django/thunderstore/api/cyberstorm/tests/test_endpoints.py @@ -142,7 +142,13 @@ def test_cyberstorm_PATCH_endpoint_schemas( user = setup_superuser_with_package(active_package_listing, package_category) api_client.force_authenticate(user) - param_values = get_parameter_values(active_package_listing) + team = user.teams.first().team + team_member = UserFactory(username="TestTeamMember") + team.add_member(user=team_member, role="member") + + param_values = get_parameter_values( + active_package_listing, username=team_member.username + ) schema = get_schema(api_client) resolver = get_resolver(schema) failures = [] diff --git a/django/thunderstore/api/cyberstorm/tests/utils.py b/django/thunderstore/api/cyberstorm/tests/utils.py index 6a5b0095e..2a9ca0d08 100644 --- a/django/thunderstore/api/cyberstorm/tests/utils.py +++ b/django/thunderstore/api/cyberstorm/tests/utils.py @@ -29,6 +29,7 @@ def get_parameter_values( if username: parameters["username"] = username + parameters["team_member"] = username return parameters diff --git a/django/thunderstore/api/cyberstorm/views/team.py b/django/thunderstore/api/cyberstorm/views/team.py index f3c8ae0dd..8a5297784 100644 --- a/django/thunderstore/api/cyberstorm/views/team.py +++ b/django/thunderstore/api/cyberstorm/views/team.py @@ -251,6 +251,7 @@ class UpdateTeamMemberAPIView(APIView): @conditional_swagger_auto_schema( operation_id="cyberstorm.team.member.update", tags=["cyberstorm"], + request_body=CyberstormTeamMemberUpdateSerializer, responses={status.HTTP_200_OK: serializer_class}, ) def patch(self, request, *args, **kwargs): From 003ffd5e4d8de30971c4e7f55a7fdfbcbedceb3e Mon Sep 17 00:00:00 2001 From: Daniel Vainio Date: Mon, 6 Oct 2025 14:21:29 +0300 Subject: [PATCH 53/61] python-packages update Refs. TS-2282 --- python-packages | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python-packages b/python-packages index 5197ec475..fbfcdec5c 160000 --- a/python-packages +++ b/python-packages @@ -1 +1 @@ -Subproject commit 5197ec475198ef6789e8fec0b6bcc5af02cd6272 +Subproject commit fbfcdec5ce0a0f706cb1cb0b85e1ddf7cfc835c4 From f2495299692395aeec12cbd27d60f0da50408f8b Mon Sep 17 00:00:00 2001 From: Daniel Vainio Date: Mon, 6 Oct 2025 14:30:29 +0300 Subject: [PATCH 54/61] Add package source endpoint Refs. TS-2282 --- .../cyberstorm/tests/test_package_source.py | 70 +++++++++++++++++++ django/thunderstore/api/urls.py | 3 + .../core/tests/test_inheritance.py | 40 +++++++++++ django/thunderstore/plugins/base.py | 4 ++ django/thunderstore/plugins/registry.py | 5 ++ 5 files changed, 122 insertions(+) create mode 100644 django/thunderstore/api/cyberstorm/tests/test_package_source.py diff --git a/django/thunderstore/api/cyberstorm/tests/test_package_source.py b/django/thunderstore/api/cyberstorm/tests/test_package_source.py new file mode 100644 index 000000000..46a122b50 --- /dev/null +++ b/django/thunderstore/api/cyberstorm/tests/test_package_source.py @@ -0,0 +1,70 @@ +import pytest +from rest_framework.test import APIClient + +from thunderstore.repository.factories import PackageFactory, PackageVersionFactory +from thunderstore.repository.models import Package + + +def get_package_source_endpoint_url(package: Package, version: str = "") -> str: + base_url = "/api/cyberstorm/package" + + package_name = package.name + namespace_id = package.namespace.name + if not version: + version = package.version_number + + return f"{base_url}/{namespace_id}/{package_name}/v/{version}/source/" + + +@pytest.mark.django_db +def test_package_source_endpoint(api_client: APIClient): + package = PackageFactory(is_active=True) + PackageVersionFactory(package=package) + url = get_package_source_endpoint_url(package) + response = api_client.get(url) + assert response.status_code == 200 + + +@pytest.mark.django_db +def test_package_not_found(api_client: APIClient): + package = PackageFactory(is_active=False) + PackageVersionFactory(package=package) + + url = get_package_source_endpoint_url(package) + response = api_client.get(url) + + assert response.status_code == 404 + assert response.json() == {"detail": "Package not found"} + + +@pytest.mark.django_db +def test_package_version_not_found(api_client: APIClient): + package = PackageFactory(is_active=True) + PackageVersionFactory(package=package) + version = package.available_versions.first() + version.version_number = "2.0.0" + version.save() + + url = get_package_source_endpoint_url(package, version="1.0.0") + response = api_client.get(url) + + assert response.status_code == 404 + assert response.json() == {"detail": "Package version not found"} + + +@pytest.mark.django_db +def test_package_source_response_format(api_client: APIClient): + package = PackageFactory(is_active=True) + PackageVersionFactory(package=package) + url = get_package_source_endpoint_url(package) + response = api_client.get(url) + + expected_result = { + "is_visible": True, + "namespace": package.namespace.name, + "package_name": package.name, + "version_number": package.version_number, + "decompilations": [], + } + + assert response.json() == expected_result diff --git a/django/thunderstore/api/urls.py b/django/thunderstore/api/urls.py index 3ff847052..c13d858e6 100644 --- a/django/thunderstore/api/urls.py +++ b/django/thunderstore/api/urls.py @@ -35,6 +35,7 @@ UpdatePackageListingCategoriesAPIView, UpdateTeamAPIView, ) +from thunderstore.plugins.registry import plugin_registry cyberstorm_urls = [ path( @@ -213,3 +214,5 @@ name="cyberstorm.user.linked-account.disconnect", ), ] + +cyberstorm_urls += plugin_registry.get_cyberstorm_api_urls() diff --git a/django/thunderstore/core/tests/test_inheritance.py b/django/thunderstore/core/tests/test_inheritance.py index fc7e4cb69..9cd1631b2 100644 --- a/django/thunderstore/core/tests/test_inheritance.py +++ b/django/thunderstore/core/tests/test_inheritance.py @@ -3,6 +3,46 @@ def test_get_effective_bool_choice_depth_first(): + assert ( + get_effective_bool_choice_depth_first( + OptionalBoolChoice.NONE, + OptionalBoolChoice.NO, + OptionalBoolChoice.YES, + ) + == OptionalBoolChoice.YES + ) + assert ( + get_effective_bool_choice_depth_first( + OptionalBoolChoice.NONE, + OptionalBoolChoice.YES, + OptionalBoolChoice.NO, + ) + == OptionalBoolChoice.NO + ) + assert ( + get_effective_bool_choice_depth_first( + OptionalBoolChoice.YES, + OptionalBoolChoice.NONE, + OptionalBoolChoice.NO, + ) + == OptionalBoolChoice.NO + ) + assert ( + get_effective_bool_choice_depth_first( + OptionalBoolChoice.YES, + OptionalBoolChoice.NO, + OptionalBoolChoice.NONE, + ) + == OptionalBoolChoice.NO + ) + assert ( + get_effective_bool_choice_depth_first( + OptionalBoolChoice.YES, + OptionalBoolChoice.NONE, + OptionalBoolChoice.NONE, + ) + == OptionalBoolChoice.YES + ) assert ( get_effective_bool_choice_depth_first( OptionalBoolChoice.NONE, diff --git a/django/thunderstore/plugins/base.py b/django/thunderstore/plugins/base.py index d072920e0..2c4278fec 100644 --- a/django/thunderstore/plugins/base.py +++ b/django/thunderstore/plugins/base.py @@ -30,6 +30,10 @@ def get_legacy_package_urls(cls) -> List[URLPattern]: def get_new_package_urls(cls) -> List[URLPattern]: return [] + @classmethod + def get_cyberstorm_api_urls(cls) -> List[URLPattern]: + return [] + @classmethod def get_package_tabs( cls, diff --git a/django/thunderstore/plugins/registry.py b/django/thunderstore/plugins/registry.py index 82c6bd7e9..7759e68ed 100644 --- a/django/thunderstore/plugins/registry.py +++ b/django/thunderstore/plugins/registry.py @@ -44,6 +44,11 @@ def get_legacy_package_urls(self) -> List[URLPattern]: def get_new_package_urls(self) -> List[URLPattern]: return list(itertools.chain(*(x.get_new_package_urls() for x in self.plugins))) + def get_cyberstorm_api_urls(self) -> List[URLPattern]: + return list( + itertools.chain(*(x.get_cyberstorm_api_urls() for x in self.plugins)) + ) + def get_package_tabs( self, user: "UserType", listing: "PackageListing" ) -> Dict[str, "PartialTab"]: From b6aa78406a0c28cf7b124807368d8d16dbb092bf Mon Sep 17 00:00:00 2001 From: Daniel Vainio Date: Mon, 6 Oct 2025 16:54:37 +0300 Subject: [PATCH 55/61] Skip tests if python-packages endpoint not available Refs. TS-2282 --- .../api/cyberstorm/tests/test_package_source.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/django/thunderstore/api/cyberstorm/tests/test_package_source.py b/django/thunderstore/api/cyberstorm/tests/test_package_source.py index 46a122b50..f2a02a5e4 100644 --- a/django/thunderstore/api/cyberstorm/tests/test_package_source.py +++ b/django/thunderstore/api/cyberstorm/tests/test_package_source.py @@ -1,9 +1,14 @@ import pytest from rest_framework.test import APIClient +from thunderstore.api.urls import cyberstorm_urls from thunderstore.repository.factories import PackageFactory, PackageVersionFactory from thunderstore.repository.models import Package +HAS_CYBERSTORM_SOURCE = "packages.api.detail.source" in [ + x.name for x in cyberstorm_urls +] + def get_package_source_endpoint_url(package: Package, version: str = "") -> str: base_url = "/api/cyberstorm/package" @@ -18,6 +23,9 @@ def get_package_source_endpoint_url(package: Package, version: str = "") -> str: @pytest.mark.django_db def test_package_source_endpoint(api_client: APIClient): + if not HAS_CYBERSTORM_SOURCE: + pytest.skip("Cyberstorm source endpoint is not enabled") + package = PackageFactory(is_active=True) PackageVersionFactory(package=package) url = get_package_source_endpoint_url(package) @@ -27,6 +35,9 @@ def test_package_source_endpoint(api_client: APIClient): @pytest.mark.django_db def test_package_not_found(api_client: APIClient): + if not HAS_CYBERSTORM_SOURCE: + pytest.skip("Cyberstorm source endpoint is not enabled") + package = PackageFactory(is_active=False) PackageVersionFactory(package=package) @@ -39,6 +50,9 @@ def test_package_not_found(api_client: APIClient): @pytest.mark.django_db def test_package_version_not_found(api_client: APIClient): + if not HAS_CYBERSTORM_SOURCE: + pytest.skip("Cyberstorm source endpoint is not enabled") + package = PackageFactory(is_active=True) PackageVersionFactory(package=package) version = package.available_versions.first() @@ -54,6 +68,9 @@ def test_package_version_not_found(api_client: APIClient): @pytest.mark.django_db def test_package_source_response_format(api_client: APIClient): + if not HAS_CYBERSTORM_SOURCE: + pytest.skip("Cyberstorm source endpoint is not enabled") + package = PackageFactory(is_active=True) PackageVersionFactory(package=package) url = get_package_source_endpoint_url(package) From 805dbf8076044fc808b5e97f7788370629c30ccc Mon Sep 17 00:00:00 2001 From: Daniel Vainio Date: Mon, 6 Oct 2025 17:42:24 +0300 Subject: [PATCH 56/61] Implement basic test for plugin_registry Implement test for checking the return types of the plugin_registry. Refs. TS-2282 --- django/thunderstore/plugins/tests/__init__.py | 0 .../plugins/tests/test_plugin_registry.py | 45 +++++++++++++++++++ 2 files changed, 45 insertions(+) create mode 100644 django/thunderstore/plugins/tests/__init__.py create mode 100644 django/thunderstore/plugins/tests/test_plugin_registry.py diff --git a/django/thunderstore/plugins/tests/__init__.py b/django/thunderstore/plugins/tests/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/django/thunderstore/plugins/tests/test_plugin_registry.py b/django/thunderstore/plugins/tests/test_plugin_registry.py new file mode 100644 index 000000000..48a213e73 --- /dev/null +++ b/django/thunderstore/plugins/tests/test_plugin_registry.py @@ -0,0 +1,45 @@ +from django.urls import URLPattern + +from thunderstore.plugins.base import BasePlugin +from thunderstore.plugins.registry import PluginRegistry, plugin_registry + + +def test_plugin_registry_return_types(): + assert isinstance(plugin_registry, PluginRegistry) + assert isinstance(plugin_registry.plugins, set) + + assert isinstance(plugin_registry.get_django_settings({}), dict) + assert isinstance(plugin_registry.get_installed_apps([]), list) + assert isinstance(plugin_registry.get_settings_urls(), list) + assert isinstance(plugin_registry.get_legacy_package_urls(), list) + assert isinstance(plugin_registry.get_new_package_urls(), list) + assert isinstance(plugin_registry.get_cyberstorm_api_urls(), list) + assert isinstance(plugin_registry.get_settings_links(), list) + + +def test_base_plugin_return_types(): + base_plugin = BasePlugin() + assert isinstance(base_plugin.get_django_settings({}), dict) + assert isinstance(base_plugin.get_settings_urls(), list) + assert isinstance(base_plugin.get_legacy_package_urls(), list) + assert isinstance(base_plugin.get_new_package_urls(), list) + assert isinstance(base_plugin.get_cyberstorm_api_urls(), list) + assert isinstance(base_plugin.get_settings_links(), list) + + +def test_plugin_registry_get_new_package_urls(): + urls = plugin_registry.get_new_package_urls() + if plugin_registry.plugins: + for url in urls: + assert isinstance(url, URLPattern) + else: + assert urls == [] + + +def test_plugin_registry_get_cyberstorm_api_urls(): + urls = plugin_registry.get_cyberstorm_api_urls() + if plugin_registry.plugins: + for url in urls: + assert isinstance(url, URLPattern) + else: + assert urls == [] From 17b201f5e366d79166618cf379759274b0970114 Mon Sep 17 00:00:00 2001 From: Daniel Vainio Date: Fri, 10 Oct 2025 12:41:44 +0300 Subject: [PATCH 57/61] Remove validation from Team.create Remove raising ValidationError in Team.create if namespace exists. This check is performed in Team.validation() and does not have to be checked in Team.create(). Do this change in order to have consistent error messaging when using the Team.create function. Refs. TS-2426 --- django/thunderstore/repository/models/team.py | 10 ++--- .../repository/tests/test_team.py | 38 ++++++++++++++++++- 2 files changed, 40 insertions(+), 8 deletions(-) diff --git a/django/thunderstore/repository/models/team.py b/django/thunderstore/repository/models/team.py index 8bff5acb8..0fbf63794 100644 --- a/django/thunderstore/repository/models/team.py +++ b/django/thunderstore/repository/models/team.py @@ -187,13 +187,9 @@ def add_member(self, user: UserType, role: str) -> TeamMember: @classmethod @transaction.atomic def create(cls, name, **kwargs): - existing_ns = Namespace.objects.filter(name__iexact=name).first() - if existing_ns: - raise ValidationError("Namespace with the Teams name exists") - else: - team = cls.objects.create(name=name, **kwargs) - Namespace.objects.create(name=name, team=team) - return team + team = cls.objects.create(name=name, **kwargs) + Namespace.objects.create(name=name, team=team) + return team @classmethod @transaction.atomic diff --git a/django/thunderstore/repository/tests/test_team.py b/django/thunderstore/repository/tests/test_team.py index 79cb2be82..9548c847e 100644 --- a/django/thunderstore/repository/tests/test_team.py +++ b/django/thunderstore/repository/tests/test_team.py @@ -75,7 +75,7 @@ def test_team_create_namespace_creation() -> None: ns.save() with pytest.raises(ValidationError) as e: Team.create(name="taken_namespace") - assert "Namespace with the Teams name exists" in str(e.value) + assert "Namespace with this name already exists" in str(e.value) @pytest.mark.django_db @@ -769,3 +769,39 @@ def test_team_ensure_user_can_manage_packages( else: assert team.can_user_manage_packages(user) is True assert team.ensure_user_can_manage_packages(user) is None + + +@pytest.mark.django_db +def test_team_create__success(): + Team.create(name="TestTeam") + assert Team.objects.filter(name="TestTeam").count() == 1 + assert Namespace.objects.filter(name="TestTeam").count() == 1 + + +@pytest.mark.django_db +def test_team_create__team_exists_fail(team): + with pytest.raises(ValidationError) as e: + Team.create(name=team.name) + assert "Team with this name already exists" in str(e.value) + assert Team.objects.filter(name=team.name).count() == 1 + assert Namespace.objects.filter(name=team.name).count() == 1 + + +@pytest.mark.django_db +def test_team_create__namespace_exists_fail(): + NamespaceFactory.create(name="TestTeam") + with pytest.raises(ValidationError) as e: + Team.create(name="TestTeam") + assert "Namespace with this name already exists" in str(e.value) + assert Team.objects.filter(name="TestTeam").count() == 0 + assert Namespace.objects.filter(name="TestTeam").count() == 1 + + +@pytest.mark.django_db +def test_team_create__team_name_read_only_fail(team): + with pytest.raises(ValidationError) as e: + team.name = "NewName" + team.save() + assert "Team name is read only" in str(e.value) + team.refresh_from_db() + assert team.name != "NewName" From c8ba27d7a29c9ffd22858bf38760b0b91ed537e3 Mon Sep 17 00:00:00 2001 From: Daniel Vainio Date: Fri, 10 Oct 2025 12:44:41 +0300 Subject: [PATCH 58/61] Update team services Update disband_team service and create_team service. Utilize Team.create() in create_team service and remove redundant validation errors. Use objects as parameters in the services instead of trying to fetch and raise 404 errors in services. Refs. TS-2426 --- .../api/cyberstorm/services/team.py | 26 ++--- .../tests/services/test_team_services.py | 96 +++++++++++++++---- 2 files changed, 89 insertions(+), 33 deletions(-) diff --git a/django/thunderstore/api/cyberstorm/services/team.py b/django/thunderstore/api/cyberstorm/services/team.py index 45d282f50..bcd732656 100644 --- a/django/thunderstore/api/cyberstorm/services/team.py +++ b/django/thunderstore/api/cyberstorm/services/team.py @@ -1,36 +1,28 @@ -from django.core.exceptions import ValidationError from django.db import transaction -from django.shortcuts import get_object_or_404 from thunderstore.account.models import ServiceAccount from thunderstore.core.exceptions import PermissionValidationError from thunderstore.core.types import UserType -from thunderstore.repository.models import Namespace, Team, TeamMember +from thunderstore.repository.models import Team, TeamMember from thunderstore.repository.models.team import TeamMemberRole @transaction.atomic -def disband_team(user: UserType, team_name: str) -> None: - teams = Team.objects.exclude(is_active=False) - team = get_object_or_404(teams, name=team_name) - team.ensure_user_can_access(user) - team.ensure_user_can_disband(user) +def disband_team(agent: UserType, team: Team) -> None: + team.ensure_user_can_access(agent) + team.ensure_user_can_disband(agent) team.delete() @transaction.atomic -def create_team(user: UserType, team_name: str) -> Team: - if not user or not user.is_authenticated or not user.is_active: +def create_team(agent: UserType, team_name: str) -> Team: + if not agent or not agent.is_authenticated or not agent.is_active: raise PermissionValidationError("Must be authenticated to create teams") - if getattr(user, "service_account", None) is not None: + if getattr(agent, "service_account", None) is not None: raise PermissionValidationError("Service accounts cannot create teams") - if Team.objects.filter(name=team_name).exists(): - raise ValidationError("A team with the provided name already exists") - if Namespace.objects.filter(name=team_name).exists(): - raise ValidationError("A namespace with the provided name already exists") - team = Team.objects.create(name=team_name) - team.add_member(user=user, role=TeamMemberRole.owner) + team = Team.create(name=team_name) + team.add_member(user=agent, role=TeamMemberRole.owner) return team diff --git a/django/thunderstore/api/cyberstorm/tests/services/test_team_services.py b/django/thunderstore/api/cyberstorm/tests/services/test_team_services.py index 8e10442b2..5b7c0a57b 100644 --- a/django/thunderstore/api/cyberstorm/tests/services/test_team_services.py +++ b/django/thunderstore/api/cyberstorm/tests/services/test_team_services.py @@ -1,6 +1,5 @@ import pytest from django.core.exceptions import ValidationError -from django.http import Http404 from conftest import TestUserTypes from thunderstore.api.cyberstorm.services import team as team_services @@ -13,45 +12,73 @@ @pytest.mark.django_db def test_disband_team_success(team_owner): team_pk = team_owner.team.pk - team_services.disband_team(team_owner.user, team_owner.team.name) + team_services.disband_team(agent=team_owner.user, team=team_owner.team) assert not Team.objects.filter(pk=team_pk).exists() -@pytest.mark.django_db -def test_disband_team_team_not_found(user): - with pytest.raises(Http404): - team_services.disband_team(user, "NonExistentTeam") - - @pytest.mark.django_db def test_disband_team_user_cannot_access_team(user): team = Team.objects.create(name="TestTeam") with pytest.raises(ValidationError, match="Must be a member to access team"): - team_services.disband_team(user, team.name) + team_services.disband_team(agent=user, team=team) @pytest.mark.django_db def test_disband_team_user_cannot_disband(team_member): with pytest.raises(ValidationError, match="Must be an owner to disband team"): - team_services.disband_team(team_member.user, team_member.team.name) + team_services.disband_team(agent=team_member.user, team=team_member.team) + + +@pytest.mark.django_db +def test_disband_team_with_packages(package, team_owner): + team = team_owner.team + package.owner = team + package.save() + + with pytest.raises(ValidationError, match="Unable to disband teams with packages"): + team_services.disband_team(agent=team_owner.user, team=team) + + +@pytest.mark.django_db +def test_disband_team_user_is_service_account(service_account, team): + service_account_user = service_account.user + with pytest.raises( + ValidationError, match="Service accounts are unable to perform this action" + ): + team_services.disband_team(agent=service_account_user, team=team) + + +@pytest.mark.django_db +def test_disband_team_user_not_authenticated(team): + with pytest.raises(ValidationError, match="Must be authenticated"): + team_services.disband_team(agent=None, team=team) + + +@pytest.mark.django_db +def test_disband_team_user_not_active(user, team): + user.is_active = False + user.save() + + with pytest.raises(ValidationError, match="User has been deactivated"): + team_services.disband_team(agent=user, team=team) @pytest.mark.django_db def test_create_team_name_exists_in_team(user): Team.objects.create(name="existing_team") - error_msg = "A team with the provided name already exists" + error_msg = "Team with this name already exists" with pytest.raises(ValidationError, match=error_msg): - team_services.create_team(user, "existing_team") + team_services.create_team(agent=user, team_name="existing_team") @pytest.mark.django_db def test_create_team_name_exists_in_namespace(user): Namespace.objects.create(name="existing_namespace") - error_msg = "A namespace with the provided name already exists" + error_msg = "Namespace with this name already exists" with pytest.raises(ValidationError, match=error_msg): - team_services.create_team(user, "existing_namespace") + team_services.create_team(agent=user, team_name="existing_namespace") @pytest.mark.django_db @@ -60,13 +87,13 @@ def test_create_team_user_is_service_account(service_account): error_msg = "Service accounts cannot create teams" with pytest.raises(ValidationError, match=error_msg): - team_services.create_team(service_account_user, "new_team") + team_services.create_team(agent=service_account_user, team_name="new_team") @pytest.mark.django_db def test_create_team_success(user): team_name = "new_team" - team = team_services.create_team(user, team_name) + team = team_services.create_team(agent=user, team_name=team_name) assert Team.objects.filter(name=team_name).exists() assert team.name == team_name @@ -330,3 +357,40 @@ def test_update_team_member_cannot_remove_last_owner(team_owner): team_services.update_team_member( team_owner.user, team_owner, TeamMemberRole.member ) + + +@pytest.mark.django_db +def test_create_team_user_not_authenticated(): + error_msg = "Must be authenticated to create teams" + with pytest.raises(ValidationError, match=error_msg): + team_services.create_team(agent=None, team_name="new_team") + + +@pytest.mark.django_db +def test_create_team_user_not_active(user): + user.is_active = False + user.save() + + error_msg = "Must be authenticated to create teams" + with pytest.raises(ValidationError, match=error_msg): + team_services.create_team(agent=user, team_name="new_team") + + +@pytest.mark.django_db +@pytest.mark.parametrize( + ("name1", "name2", "should_fail"), + ( + ("Team", "team", True), + ("Team", "t_eam", False), + ("team", "teaM", True), + ("team", "team", True), + ), +) +def test_create_team_name_conflict(user, name1: str, name2: str, should_fail: bool): + Team.create(name=name1) + if should_fail: + with pytest.raises(ValidationError): + team_services.create_team(agent=user, team_name=name2) + else: + team = team_services.create_team(agent=user, team_name=name2) + assert team.name == name2 From 62751e63e808c7ee5d7960a2073242ceee622670 Mon Sep 17 00:00:00 2001 From: Daniel Vainio Date: Fri, 10 Oct 2025 12:46:00 +0300 Subject: [PATCH 59/61] Update create team and disband team views Update the create team and disband team views since the parameter names changed of the services. Refs. TS-2426 --- .../thunderstore/api/cyberstorm/tests/test_create_team.py | 6 ++---- django/thunderstore/api/cyberstorm/views/team.py | 8 +++++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/django/thunderstore/api/cyberstorm/tests/test_create_team.py b/django/thunderstore/api/cyberstorm/tests/test_create_team.py index 149e1b883..15d1c0847 100644 --- a/django/thunderstore/api/cyberstorm/tests/test_create_team.py +++ b/django/thunderstore/api/cyberstorm/tests/test_create_team.py @@ -53,9 +53,7 @@ def test_create_team__fail_because_team_with_provided_name_exists( ): api_client.force_authenticate(user) response = make_request(api_client, team.name) - expected_response = { - "non_field_errors": ["A team with the provided name already exists"] - } + expected_response = {"non_field_errors": ["Team with this name already exists"]} assert response.status_code == 400 assert response.json() == expected_response @@ -70,7 +68,7 @@ def test_create_team__fail_because_team_with_provided_namespace_exists( NamespaceFactory(name="CoolestTeamNameEver") response = make_request(api_client, "CoolestTeamNameEver") expected_response = { - "non_field_errors": ["A namespace with the provided name already exists"] + "non_field_errors": ["Namespace with this name already exists"] } assert response.status_code == 400 diff --git a/django/thunderstore/api/cyberstorm/views/team.py b/django/thunderstore/api/cyberstorm/views/team.py index 8a5297784..2c7327ddf 100644 --- a/django/thunderstore/api/cyberstorm/views/team.py +++ b/django/thunderstore/api/cyberstorm/views/team.py @@ -75,8 +75,10 @@ class TeamCreateAPIView(APIView): def post(self, request, *args, **kwargs): serializer = CyberstormCreateTeamSerializer(data=request.data) serializer.is_valid(raise_exception=True) + team_name = serializer.validated_data["name"] - team = create_team(user=request.user, team_name=team_name) + team = create_team(agent=request.user, team_name=team_name) + return_data = CyberstormTeamSerializer(team).data return Response(return_data, status=status.HTTP_201_CREATED) @@ -166,8 +168,8 @@ class DisbandTeamAPIView(APIView): responses={status.HTTP_204_NO_CONTENT: ""}, ) def delete(self, request, *args, **kwargs): - team_name = kwargs["team_name"] - disband_team(user=request.user, team_name=team_name) + team = get_object_or_404(Team, name=kwargs["team_name"]) + disband_team(agent=request.user, team=team) return Response(status=status.HTTP_204_NO_CONTENT) From 70cede3067b3abaf32f1184885f0282c1c674064 Mon Sep 17 00:00:00 2001 From: Daniel Vainio Date: Fri, 10 Oct 2025 12:47:21 +0300 Subject: [PATCH 60/61] Use services in create/disband form views Use the create_team and disband_team services in the forms and views responsible for disband teams and creating teams. Update tests accordingly. Update the form for creating teams to clean the name manually in order to keep the error messaging consistent, otherwise tests will fail since because it is a ModelForm which throws a different formatted error by default. Refs. TS-2426 --- django/thunderstore/repository/forms/team.py | 47 +++++++++---------- .../repository/tests/test_team_forms.py | 10 +++- .../repository/views/team_settings.py | 10 +++- 3 files changed, 40 insertions(+), 27 deletions(-) diff --git a/django/thunderstore/repository/forms/team.py b/django/thunderstore/repository/forms/team.py index f55066689..6cf5960e3 100644 --- a/django/thunderstore/repository/forms/team.py +++ b/django/thunderstore/repository/forms/team.py @@ -3,21 +3,17 @@ from django import forms from django.contrib.auth import get_user_model from django.core.exceptions import ObjectDoesNotExist, ValidationError +from django.db import transaction from thunderstore.api.cyberstorm.services.team import ( + create_team, + disband_team, remove_team_member, update_team, update_team_member, ) -from thunderstore.core.exceptions import PermissionValidationError from thunderstore.core.types import UserType -from thunderstore.repository.models import ( - Namespace, - Team, - TeamMember, - TeamMemberRole, - transaction, -) +from thunderstore.repository.models import Team, TeamMember, TeamMemberRole from thunderstore.repository.validators import PackageReferenceComponentValidator User = get_user_model() @@ -38,23 +34,22 @@ def __init__(self, user: UserType, *args, **kwargs): def clean_name(self): name = self.cleaned_data["name"] - if Team.objects.filter(name__iexact=name.lower()).exists(): - raise ValidationError(f"A team with the provided name already exists") - if Namespace.objects.filter(name__iexact=name.lower()).exists(): - raise ValidationError("A namespace with the provided name already exists") + if Team.objects.filter(name__iexact=name).exists(): + raise ValidationError("Team with this name already exists") return name - def clean(self): - if not self.user or not self.user.is_authenticated or not self.user.is_active: - raise PermissionValidationError("Must be authenticated to create teams") - if getattr(self.user, "service_account", None) is not None: - raise PermissionValidationError("Service accounts cannot create teams") - return super().clean() - @transaction.atomic def save(self, *args, **kwargs) -> Team: - instance = super().save() - instance.add_member(user=self.user, role=TeamMemberRole.owner) + if self.errors: + raise ValidationError(self.errors) + + try: + team_name = self.cleaned_data["name"] + instance = create_team(agent=self.user, team_name=team_name) + except ValidationError as e: + self.add_error(None, e) + raise ValidationError(self.errors) + return instance @@ -169,13 +164,17 @@ def clean_verification(self): def clean(self): if not self.instance.pk: raise ValidationError("Missing team instance") - self.instance.ensure_user_can_disband(self.user) return super().clean() @transaction.atomic def save(self, **kwargs): - self.instance.ensure_user_can_disband(self.user) - self.instance.delete() + if self.errors: + raise ValidationError(self.errors) + try: + disband_team(agent=self.user, team=self.instance) + except ValidationError as e: + self.add_error(None, e) + raise ValidationError(self.errors) class DonationLinkTeamForm(forms.ModelForm): diff --git a/django/thunderstore/repository/tests/test_team_forms.py b/django/thunderstore/repository/tests/test_team_forms.py index 3771c0597..840067a9a 100644 --- a/django/thunderstore/repository/tests/test_team_forms.py +++ b/django/thunderstore/repository/tests/test_team_forms.py @@ -40,6 +40,8 @@ def test_form_create_team_valid_data(user_type: str) -> None: data={"name": "TeamName"}, ) if expected_error: + with pytest.raises(ValidationError): + form.save() assert form.is_valid() is False assert expected_error in str(repr(form.errors)) else: @@ -71,8 +73,10 @@ def test_form_create_team_team_name_conflict( data={"name": name2}, ) if should_fail: + with pytest.raises(ValidationError): + form.save() assert form.is_valid() is False - assert "A team with the provided name already exists" in str(repr(form.errors)) + assert "Team with this name already exists" in str(repr(form.errors)) else: assert form.is_valid() is True team = form.save() @@ -461,6 +465,8 @@ def test_form_disband_team_form( assert form.save() is None assert Team.objects.filter(pk=team.pk).exists() is False else: + with pytest.raises(ValidationError): + form.save() assert form.is_valid() is False assert form.errors @@ -497,6 +503,8 @@ def test_form_disband_team_form_packages_exist( instance=team, data={"verification": team.name}, ) + with pytest.raises(ValidationError): + form.save() assert form.is_valid() is False assert "Unable to disband teams with packages" in str(repr(form.errors)) diff --git a/django/thunderstore/repository/views/team_settings.py b/django/thunderstore/repository/views/team_settings.py index 1dc83c3be..9f8f5adea 100644 --- a/django/thunderstore/repository/views/team_settings.py +++ b/django/thunderstore/repository/views/team_settings.py @@ -153,7 +153,10 @@ def get_context_data(self, **kwargs): @transaction.atomic def form_valid(self, form): - instance = form.save() + try: + instance = form.save() + except ValidationError: + return super().form_invalid(form) return redirect(instance.settings_url) @@ -175,7 +178,10 @@ def get_form_kwargs(self): @transaction.atomic def form_valid(self, form): - form.save() + try: + form.save() + except ValidationError: + return self.form_invalid(form) return redirect(reverse("settings.teams")) From 988b7e85d1a6729c9a02a86ba03cf599fe25fd1e Mon Sep 17 00:00:00 2001 From: Daniel Vainio Date: Tue, 14 Oct 2025 10:56:56 +0300 Subject: [PATCH 61/61] Update test_team_services error validation Some actions raise PermissionValidationError instead of ValidationError. Although PermissionValidationError inherits from ValidationError, tests that specifically expect these errors should use PermissionValidationError to ensure accurate validation. Refs. TS-2426 --- .../tests/services/test_team_services.py | 23 +++++++++++-------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/django/thunderstore/api/cyberstorm/tests/services/test_team_services.py b/django/thunderstore/api/cyberstorm/tests/services/test_team_services.py index 5b7c0a57b..4304924c5 100644 --- a/django/thunderstore/api/cyberstorm/tests/services/test_team_services.py +++ b/django/thunderstore/api/cyberstorm/tests/services/test_team_services.py @@ -19,13 +19,15 @@ def test_disband_team_success(team_owner): @pytest.mark.django_db def test_disband_team_user_cannot_access_team(user): team = Team.objects.create(name="TestTeam") - with pytest.raises(ValidationError, match="Must be a member to access team"): + error_msg = "Must be a member to access team" + with pytest.raises(PermissionValidationError, match=error_msg): team_services.disband_team(agent=user, team=team) @pytest.mark.django_db def test_disband_team_user_cannot_disband(team_member): - with pytest.raises(ValidationError, match="Must be an owner to disband team"): + error_msg = "Must be an owner to disband team" + with pytest.raises(PermissionValidationError, match=error_msg): team_services.disband_team(agent=team_member.user, team=team_member.team) @@ -42,15 +44,15 @@ def test_disband_team_with_packages(package, team_owner): @pytest.mark.django_db def test_disband_team_user_is_service_account(service_account, team): service_account_user = service_account.user - with pytest.raises( - ValidationError, match="Service accounts are unable to perform this action" - ): + error_msg = "Service accounts are unable to perform this action" + with pytest.raises(PermissionValidationError, match=error_msg): team_services.disband_team(agent=service_account_user, team=team) @pytest.mark.django_db def test_disband_team_user_not_authenticated(team): - with pytest.raises(ValidationError, match="Must be authenticated"): + error_msg = "Must be authenticated" + with pytest.raises(PermissionValidationError, match=error_msg): team_services.disband_team(agent=None, team=team) @@ -59,7 +61,8 @@ def test_disband_team_user_not_active(user, team): user.is_active = False user.save() - with pytest.raises(ValidationError, match="User has been deactivated"): + error_msg = "User has been deactivated" + with pytest.raises(PermissionValidationError, match=error_msg): team_services.disband_team(agent=user, team=team) @@ -86,7 +89,7 @@ def test_create_team_user_is_service_account(service_account): service_account_user = service_account.user error_msg = "Service accounts cannot create teams" - with pytest.raises(ValidationError, match=error_msg): + with pytest.raises(PermissionValidationError, match=error_msg): team_services.create_team(agent=service_account_user, team_name="new_team") @@ -362,7 +365,7 @@ def test_update_team_member_cannot_remove_last_owner(team_owner): @pytest.mark.django_db def test_create_team_user_not_authenticated(): error_msg = "Must be authenticated to create teams" - with pytest.raises(ValidationError, match=error_msg): + with pytest.raises(PermissionValidationError, match=error_msg): team_services.create_team(agent=None, team_name="new_team") @@ -372,7 +375,7 @@ def test_create_team_user_not_active(user): user.save() error_msg = "Must be authenticated to create teams" - with pytest.raises(ValidationError, match=error_msg): + with pytest.raises(PermissionValidationError, match=error_msg): team_services.create_team(agent=user, team_name="new_team")