Skip to content

Commit ee55dfd

Browse files
committed
Refactor package rating API changes
- Remove form usage entirely as it was complicating things for no benefit - Create a classmethod in the PackageRating model for performing the rating action (and validation of it) - Update the v1 rating API view to use the new classmethod for performing the action - Move experimental API package rating list to it's own dedicated endpoint to avoid slowing down the main current-user endpoint if there are lots of ratings - Cap the rated package count returned by the experimental API to improve performance for users with hundreds of rated packages. This should be updated to a paginated + local cache solution later on, good enough for now.
1 parent ed973af commit ee55dfd

File tree

15 files changed

+219
-260
lines changed

15 files changed

+219
-260
lines changed

django/thunderstore/api/cyberstorm/tests/test_package_rating.py

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,13 @@
1010
@pytest.mark.django_db
1111
def test_package_rating_api_view__succeeds(
1212
api_client: APIClient,
13-
package: Package,
13+
active_package: Package,
1414
user: UserType,
1515
) -> None:
1616
api_client.force_authenticate(user)
1717

1818
response = api_client.post(
19-
f"/api/cyberstorm/package/{package.namespace}/{package.name}/rate/",
19+
f"/api/cyberstorm/package/{active_package.namespace}/{active_package.name}/rate/",
2020
json.dumps({"target_state": "rated"}),
2121
content_type="application/json",
2222
)
@@ -26,7 +26,7 @@ def test_package_rating_api_view__succeeds(
2626
assert actual["score"] == 1
2727

2828
response = api_client.post(
29-
f"/api/cyberstorm/package/{package.namespace}/{package.name}/rate/",
29+
f"/api/cyberstorm/package/{active_package.namespace}/{active_package.name}/rate/",
3030
json.dumps({"target_state": "unrated"}),
3131
content_type="application/json",
3232
)
@@ -55,10 +55,10 @@ def test_package_rating_api_view__returns_error_for_non_existent_package(
5555
@pytest.mark.django_db
5656
def test_package_rating_api_view__returns_error_for_no_user(
5757
api_client: APIClient,
58-
package: Package,
58+
active_package: Package,
5959
) -> None:
6060
response = api_client.post(
61-
f"/api/cyberstorm/package/{package.namespace}/{package.name}/rate/",
61+
f"/api/cyberstorm/package/{active_package.namespace}/{active_package.name}/rate/",
6262
json.dumps({"target_state": "rated"}),
6363
content_type="application/json",
6464
)
@@ -70,15 +70,13 @@ def test_package_rating_api_view__returns_error_for_no_user(
7070
@pytest.mark.django_db
7171
def test_package_rating_api_view__returns_error_for_bad_data(
7272
api_client: APIClient,
73-
package: Package,
73+
active_package: Package,
7474
user: UserType,
7575
) -> None:
7676
api_client.force_authenticate(user)
77-
package.is_active = False
78-
package.save()
7977

8078
response = api_client.post(
81-
f"/api/cyberstorm/package/{package.namespace}/{package.name}/rate/",
79+
f"/api/cyberstorm/package/{active_package.namespace}/{active_package.name}/rate/",
8280
json.dumps({"bad_data": "rated"}),
8381
content_type="application/json",
8482
)
@@ -87,10 +85,10 @@ def test_package_rating_api_view__returns_error_for_bad_data(
8785
assert actual["target_state"] == ["This field is required."]
8886

8987
response = api_client.post(
90-
f"/api/cyberstorm/package/{package.namespace}/{package.name}/rate/",
88+
f"/api/cyberstorm/package/{active_package.namespace}/{active_package.name}/rate/",
9189
json.dumps({"target_state": "bad"}),
9290
content_type="application/json",
9391
)
9492
actual = response.json()
9593

96-
assert actual["__all__"] == ["Given target_state is invalid"]
94+
assert actual["non_field_errors"] == ["Invalid target_state"]

django/thunderstore/api/cyberstorm/views/__init__.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
PackageListingByDependencyListAPIView,
99
PackageListingByNamespaceListAPIView,
1010
)
11-
from .package_rating import PackageRatingRateAPIView
11+
from .package_rating import RatePackageAPIView
1212
from .package_version_list import PackageVersionListAPIView
1313
from .team import (
1414
TeamAPIView,
@@ -32,5 +32,5 @@
3232
"TeamMemberAddAPIView",
3333
"TeamMemberListAPIView",
3434
"TeamServiceAccountListAPIView",
35-
"PackageRatingRateAPIView",
35+
"RatePackageAPIView",
3636
]
Lines changed: 22 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,53 +1,51 @@
11
from django.http import HttpRequest
22
from rest_framework import serializers
3-
from rest_framework.exceptions import ValidationError
43
from rest_framework.generics import get_object_or_404
54
from rest_framework.permissions import IsAuthenticated
65
from rest_framework.response import Response
76
from rest_framework.views import APIView
87

98
from thunderstore.api.utils import conditional_swagger_auto_schema
10-
from thunderstore.repository.forms import RateForm
11-
from thunderstore.repository.models import Package
9+
from thunderstore.repository.models import Package, PackageRating
1210

1311

14-
class CyberstormPackageRatingRateRequestSerialiazer(serializers.Serializer):
12+
class CyberstormRatePackageRequestSerialiazer(serializers.Serializer):
1513
target_state = serializers.CharField()
1614

1715

18-
class CyberstormPackageRatingRateResponseSerialiazer(serializers.Serializer):
16+
class CyberstormRatePackageResponseSerialiazer(serializers.Serializer):
1917
state = serializers.CharField()
2018
score = serializers.IntegerField()
2119

2220

23-
class PackageRatingRateAPIView(APIView):
21+
class RatePackageAPIView(APIView):
2422
permission_classes = [IsAuthenticated]
2523

2624
@conditional_swagger_auto_schema(
27-
request_body=CyberstormPackageRatingRateRequestSerialiazer,
28-
responses={200: CyberstormPackageRatingRateResponseSerialiazer},
29-
operation_id="cyberstorm.package_rating.rate",
25+
request_body=CyberstormRatePackageRequestSerialiazer,
26+
responses={200: CyberstormRatePackageResponseSerialiazer},
27+
operation_id="cyberstorm.package.rate",
3028
tags=["cyberstorm"],
3129
)
3230
def post(self, request: HttpRequest, namespace_id: str, package_name: str):
33-
serializer = CyberstormPackageRatingRateRequestSerialiazer(data=request.data)
31+
serializer = CyberstormRatePackageRequestSerialiazer(data=request.data)
3432
serializer.is_valid(raise_exception=True)
3533
package = get_object_or_404(
36-
Package,
34+
Package.objects.active(),
3735
namespace__name=namespace_id,
38-
name__iexact=package_name,
36+
name=package_name,
3937
)
40-
form = RateForm(
41-
user=request.user,
38+
result_state = PackageRating.rate_package(
39+
agent=request.user,
4240
package=package,
43-
data=serializer.validated_data,
41+
target_state=serializer.validated_data["target_state"],
42+
)
43+
package = Package.objects.get(pk=package.pk)
44+
return Response(
45+
CyberstormRatePackageResponseSerialiazer(
46+
{
47+
"state": result_state,
48+
"score": package.rating_score,
49+
}
50+
).data
4451
)
45-
if form.is_valid():
46-
(result_state, score) = form.execute()
47-
return Response(
48-
CyberstormPackageRatingRateResponseSerialiazer(
49-
{"state": result_state, "score": score}
50-
).data
51-
)
52-
else:
53-
raise ValidationError(form.errors)

django/thunderstore/api/urls.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,10 @@
88
PackageListingByCommunityListAPIView,
99
PackageListingByDependencyListAPIView,
1010
PackageListingByNamespaceListAPIView,
11-
PackageRatingRateAPIView,
1211
PackageVersionChangelogAPIView,
1312
PackageVersionListAPIView,
1413
PackageVersionReadmeAPIView,
14+
RatePackageAPIView,
1515
TeamAPIView,
1616
TeamMemberAddAPIView,
1717
TeamMemberListAPIView,
@@ -81,8 +81,8 @@
8181
),
8282
path(
8383
"package/<str:namespace_id>/<str:package_name>/rate/",
84-
PackageRatingRateAPIView.as_view(),
85-
name="cyberstorm.package_rating.rate",
84+
RatePackageAPIView.as_view(),
85+
name="cyberstorm.package.rate",
8686
),
8787
path(
8888
"team/<str:team_id>/",

django/thunderstore/core/types.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,12 @@
88

99
from thunderstore.account.models import UserSettings
1010
from thunderstore.community.models import Community
11-
from thunderstore.repository.models import Team
11+
from thunderstore.repository.models import PackageRating, Team
1212

1313
class UserType(User):
1414
teams: "Manager[Team]"
1515
settings: Optional[UserSettings]
16+
package_ratings: "Manager[PackageRating]"
1617

1718
class HttpRequestType(HttpRequest):
1819
community: "Optional[Community]"

django/thunderstore/repository/api/experimental/urls.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,19 @@
2626
PackageWikiListAPIView,
2727
)
2828
from thunderstore.social.api.experimental.views import CurrentUserExperimentalApiView
29+
from thunderstore.social.api.experimental.views.current_user import (
30+
CurrentUserRatedPackagesExperimentalApiView,
31+
)
2932

3033
urls = [
3134
path(
3235
"current-user/", CurrentUserExperimentalApiView.as_view(), name="current-user"
3336
),
37+
path(
38+
"current-user/rated-packages/",
39+
CurrentUserRatedPackagesExperimentalApiView.as_view(),
40+
name="current-user.rated-packages",
41+
),
3442
path("package-index/", PackageIndexApiView.as_view(), name="package-index"),
3543
path("package/", PackageListApiView.as_view(), name="package-list"),
3644
path("package/wikis/", PackageWikiListAPIView.as_view(), name="package-wiki-list"),

django/thunderstore/repository/api/v1/viewsets.py

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
from thunderstore.repository.mixins import CommunityMixin
2525
from thunderstore.repository.models import Package, PackageRating
2626
from thunderstore.repository.models.cache import APIV1PackageCache
27-
from thunderstore.repository.permissions import ensure_can_rate_package
2827
from thunderstore.utils.batch import batch
2928

3029
PACKAGE_SERIALIZER = PackageListingSerializer
@@ -134,15 +133,10 @@ def rate(
134133
community_identifier: Optional[str] = None,
135134
) -> Response:
136135
package = get_object_or_404(Package.objects.active(), uuid4=uuid4)
137-
user = request.user
138-
ensure_can_rate_package(user, package)
139-
target_state = request.data.get("target_state")
140-
if target_state == "rated":
141-
PackageRating.objects.get_or_create(rater=user, package=package)
142-
result_state = "rated"
143-
else:
144-
PackageRating.objects.filter(rater=user, package=package).delete()
145-
result_state = "unrated"
136+
result_state = PackageRating.rate_package(
137+
request.user, package, request.data.get("target_state")
138+
)
139+
package = Package.objects.get(pk=package.pk)
146140
return Response(
147141
{
148142
"state": result_state,
Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1 @@
1-
from .package_rating import *
21
from .team import *

django/thunderstore/repository/forms/package_rating.py

Lines changed: 0 additions & 38 deletions
This file was deleted.

django/thunderstore/repository/models/package_rating.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,15 @@
1+
from typing import TYPE_CHECKING, Literal, Optional, Union
2+
13
from django.conf import settings
4+
from django.core.exceptions import ValidationError
25
from django.db import models
36

7+
from thunderstore.core.types import UserType
8+
from thunderstore.repository.permissions import ensure_can_rate_package
9+
10+
if TYPE_CHECKING:
11+
from thunderstore.repository.models import Package
12+
413

514
class PackageRating(models.Model):
615
rater = models.ForeignKey(
@@ -26,3 +35,22 @@ class Meta:
2635

2736
def __str__(self):
2837
return f"{self.rater.username} rating on {self.package.full_package_name}"
38+
39+
@classmethod
40+
def rate_package(
41+
cls,
42+
agent: Optional[UserType],
43+
package: "Package",
44+
target_state: Union[Literal["rated"], Literal["unrated"]],
45+
) -> Union[Literal["rated"], Literal["unrated"]]:
46+
if target_state not in ("rated", "unrated"):
47+
raise ValidationError("Invalid target_state")
48+
49+
ensure_can_rate_package(agent, package)
50+
51+
if target_state == "rated":
52+
PackageRating.objects.get_or_create(rater=agent, package=package)
53+
return "rated"
54+
else:
55+
PackageRating.objects.filter(rater=agent, package=package).delete()
56+
return "unrated"

0 commit comments

Comments
 (0)