Merge pull request #985 from thunderstore-io/internal-notes

Improve package review tool & status messages

builder/src/api/api.ts

@@ -84,14 +84,23 @@ class ExperimentalApiImpl extends ThunderstoreApi {
         return (await response.json()) as UpdatePackageListingResponse;
     };
 
-    approvePackageListing = async (props: { packageListingId: string }) => {
-        await this.post(ApiUrls.approvePackageListing(props.packageListingId));
+    approvePackageListing = async (props: {
+        packageListingId: string;
+        data: {
+            internal_notes: string;
+        };
+    }) => {
+        await this.post(
+            ApiUrls.approvePackageListing(props.packageListingId),
+            props.data
+        );
     };
 
     rejectPackageListing = async (props: {
         packageListingId: string;
         data: {
             rejection_reason: string;
+            internal_notes: string;
         };
     }) => {
         await this.post(

builder/src/components/PackageReview/Context.tsx

@@ -5,6 +5,7 @@ export type ContextProps = {
     reviewStatus: ReviewStatus;
     rejectionReason: string;
     packageListingId: string;
+    internalNotes: string;
 };
 
 export interface IReviewContext {

builder/src/components/PackageReview/Modal.tsx

@@ -54,6 +54,14 @@ const Body: React.FC<BodyProps> = (props) => {
                         style={{ minHeight: "100px" }}
                     />
                 </div>
+                <div className="mt-3">
+                    <h6>Internal notes</h6>
+                    <textarea
+                        {...props.form.control.register("internalNotes")}
+                        className={"code-input"}
+                        style={{ minHeight: "100px" }}
+                    />
+                </div>
             </form>
             {props.form.error && (
                 <div className={"alert alert-danger mt-2 mb-0"}>
@@ -102,10 +110,7 @@ const Footer: React.FC<FooterProps> = (props) => {
 };
 export const PackageReviewModal: React.FC = () => {
     const context = useReviewContext();
-    const form = usePackageReviewForm(
-        context.props.packageListingId,
-        context.props.rejectionReason
-    );
+    const form = usePackageReviewForm(context.props);
     useOnEscape(context.closeModal);
 
     const style = {

builder/src/components/PackageReview/useForm.ts

@@ -7,6 +7,7 @@ import { Control } from "react-hook-form/dist/types";
 type Status = undefined | "SUBMITTING" | "SUCCESS" | "ERROR";
 export type PackageListingReviewFormValues = {
     rejectionReason: string;
+    internalNotes: string;
 };
 
 export type PackageListingReviewForm = {
@@ -17,13 +18,19 @@ export type PackageListingReviewForm = {
     status: Status;
 };
 
-export const usePackageReviewForm = (
-    packageListingId: string,
-    rejectionReason?: string,
-    onSuccess?: () => void
-): PackageListingReviewForm => {
+export const usePackageReviewForm = (props: {
+    packageListingId: string;
+    rejectionReason?: string;
+    internalNotes?: string;
+    onSuccess?: () => void;
+}): PackageListingReviewForm => {
+    const { packageListingId, rejectionReason, internalNotes } = props;
+
     const { handleSubmit, control } = useForm<PackageListingReviewFormValues>({
-        defaultValues: { rejectionReason },
+        defaultValues: {
+            rejectionReason,
+            internalNotes,
+        },
     });
     const [status, setStatus] = useState<Status>(undefined);
     const [error, setError] = useState<string | undefined>(undefined);
@@ -45,22 +52,28 @@ export const usePackageReviewForm = (
         [setError, setStatus]
     );
 
-    const approve = handleSubmit(async () => {
+    const approve = handleSubmit(async (data) => {
         await handleState(async () => {
             await ExperimentalApi.approvePackageListing({
                 packageListingId: packageListingId,
+                data: {
+                    internal_notes: data.internalNotes,
+                },
             });
-            if (onSuccess) onSuccess();
+            if (props.onSuccess) props.onSuccess();
         });
     });
 
     const reject = handleSubmit(async (data) => {
         await handleState(async () => {
             await ExperimentalApi.rejectPackageListing({
                 packageListingId: packageListingId,
-                data: { rejection_reason: data.rejectionReason },
+                data: {
+                    rejection_reason: data.rejectionReason,
+                    internal_notes: data.internalNotes,
+                },
             });
-            if (onSuccess) onSuccess();
+            if (props.onSuccess) props.onSuccess();
         });
     });
 

django/thunderstore/community/api/experimental/tests/test_api_approval_status.py

@@ -31,15 +31,18 @@ def test_api_experimental_package_listing_approve_success(
     active_package_listing: PackageListing,
     moderator: UserType,
 ):
+    notes = "Internal note"
     api_client.force_authenticate(user=moderator)
     response = api_client.post(
         f"/api/experimental/package-listing/{active_package_listing.pk}/approve/",
+        data=json.dumps({"internal_notes": notes}),
         content_type="application/json",
     )
 
     assert response.status_code == 200
     active_package_listing.refresh_from_db()
     assert active_package_listing.review_status == PackageListingReviewStatus.approved
+    assert active_package_listing.notes == notes
 
 
 @pytest.mark.django_db
@@ -72,17 +75,24 @@ def test_api_experimental_package_listing_reject_success(
     moderator: UserType,
 ):
     reason = "Bad upload"
+    notes = "Internal note"
     api_client.force_authenticate(user=moderator)
     response = api_client.post(
         f"/api/experimental/package-listing/{active_package_listing.pk}/reject/",
-        data=json.dumps({"rejection_reason": reason}),
+        data=json.dumps(
+            {
+                "rejection_reason": reason,
+                "internal_notes": notes,
+            }
+        ),
         content_type="application/json",
     )
 
     assert response.status_code == 200
     active_package_listing.refresh_from_db()
     assert active_package_listing.review_status == PackageListingReviewStatus.rejected
     assert active_package_listing.rejection_reason == reason
+    assert active_package_listing.notes == notes
 
 
 @pytest.mark.django_db

django/thunderstore/community/api/experimental/views/listing.py

@@ -49,6 +49,11 @@ def post(self, request, *args, **kwargs):
 
 class PackageListingRejectRequestSerializer(serializers.Serializer):
     rejection_reason = serializers.CharField()
+    internal_notes = serializers.CharField(
+        allow_blank=True,
+        allow_null=True,
+        required=False,
+    )
 
 
 class PackageListingRejectApiView(GenericAPIView):
@@ -70,6 +75,7 @@ def post(self, request, *args, **kwargs):
             listing.reject(
                 agent=request.user,
                 rejection_reason=params["rejection_reason"],
+                internal_notes=params.get("internal_notes"),
             )
             get_package_listing_or_404.clear_cache_with_args(
                 namespace=listing.package.namespace.name,
@@ -81,6 +87,14 @@ def post(self, request, *args, **kwargs):
             raise PermissionDenied()
 
 
+class PackageListingApproveRequestSerializer(serializers.Serializer):
+    internal_notes = serializers.CharField(
+        allow_blank=True,
+        allow_null=True,
+        required=False,
+    )
+
+
 class PackageListingApproveApiView(GenericAPIView):
     queryset = PackageListing.objects.select_related("community", "package")
 
@@ -91,8 +105,15 @@ class PackageListingApproveApiView(GenericAPIView):
     def post(self, request, *args, **kwargs):
         listing: PackageListing = self.get_object()
 
+        request_serializer = PackageListingApproveRequestSerializer(data=request.data)
+        request_serializer.is_valid(raise_exception=True)
+        params = request_serializer.validated_data
+
         try:
-            listing.approve(agent=request.user)
+            listing.approve(
+                agent=request.user,
+                internal_notes=params.get("internal_notes"),
+            )
             get_package_listing_or_404.clear_cache_with_args(
                 namespace=listing.package.namespace.name,
                 name=listing.package.name,

django/thunderstore/community/models/package_listing.py

@@ -160,31 +160,55 @@ def build_audit_event(
 
     @transaction.atomic
     def reject(
-        self, agent: Optional[UserType], rejection_reason: str, is_system: bool = False
+        self,
+        agent: Optional[UserType],
+        rejection_reason: str,
+        is_system: bool = False,
+        internal_notes: Optional[str] = None,
     ):
         if is_system or self.can_user_manage_approval_status(agent):
             self.rejection_reason = rejection_reason
             self.review_status = PackageListingReviewStatus.rejected
-            self.save()
+            self.notes = internal_notes or self.notes
+            self.save(
+                update_fields=(
+                    "rejection_reason",
+                    "review_status",
+                    "notes",
+                )
+            )
+            message = "\n\n".join(filter(bool, (rejection_reason, internal_notes)))
             fire_audit_event(
                 self.build_audit_event(
                     action=AuditAction.PACKAGE_REJECTED,
                     user_id=agent.pk if agent else None,
-                    message=rejection_reason,
+                    message=message,
                 )
             )
         else:
             raise PermissionError()
 
     @transaction.atomic
-    def approve(self, agent: Optional[UserType], is_system: bool = False):
+    def approve(
+        self,
+        agent: Optional[UserType],
+        is_system: bool = False,
+        internal_notes: Optional[str] = None,
+    ):
         if is_system or self.can_user_manage_approval_status(agent):
             self.review_status = PackageListingReviewStatus.approved
-            self.save()
+            self.notes = internal_notes or self.notes
+            self.save(
+                update_fields=(
+                    "review_status",
+                    "notes",
+                )
+            )
             fire_audit_event(
                 self.build_audit_event(
                     action=AuditAction.PACKAGE_APPROVED,
                     user_id=agent.pk if agent else None,
+                    message=internal_notes,
                 )
             )
         else:

django/thunderstore/community/templates/community/packagelisting_detail.html

@@ -48,6 +48,7 @@
 );
 </script>
 {% endif %}
+
 {% cache_until "any_package_updated" "mod-detail-header" 300 object.package.pk community_identifier %}
 
 <nav class="mt-3" aria-label="breadcrumb">
@@ -65,14 +66,18 @@
     </div>
 {% endif %}
 
-{% if object.is_rejected %}
+{% endcache %}
+
+{% if show_review_status and object.is_rejected %}
 <div class="card text-white bg-danger mt-2">
     <div class="card-body">
         <h4 class="card-title">
             Package rejected
         </h4>
         <p class="card-text">
-            This package has been rejected by site or community moderators
+            This package has been rejected by site or community moderators.
+            If you think this is a mistake, please reach out to the moderators in
+            <a href="https://discord.thunderstore.io/">our Discord server</a>
         </p>
         {% if object.rejection_reason %}
         <p class="card-text">
@@ -83,7 +88,7 @@ <h4 class="card-title">
 </div>
 {% endif %}
 
-{% if object.is_waiting_for_approval %}
+{% if show_review_status and object.is_waiting_for_approval %}
 <div class="card text-white bg-warning mt-2">
     <div class="card-body">
         <h4 class="card-title">
@@ -96,8 +101,18 @@ <h4 class="card-title">
 </div>
 {% endif %}
 
+{% if show_internal_notes and object.notes %}
+    <div class="card text-white bg-info mt-2">
+        <div class="card-body">
+            <h4 class="card-title">
+                Internal notes
+            </h4>
+            <p class="card-text">{{ object.notes }}</p>
+        </div>
+    </div>
+{% endif %}
+
 <div class="card bg-light mt-2">
-    {% endcache %}
     {% include "community/includes/package_tabs.html" with tabs=tabs %}
     {% cache_until "any_package_updated" "mod-detail-content" 300 object.package.pk community_identifier %}
     <div class="card-header">

django/thunderstore/repository/views/repository.py

@@ -480,7 +480,7 @@ def get_object(self, *args, **kwargs) -> PackageListing:
             raise Http404("Package is waiting for approval or has been rejected")
         return listing
 
-    @property
+    @cached_property
     def can_manage(self):
         return any(
             (
@@ -494,7 +494,7 @@ def can_manage(self):
     def can_manage_deprecation(self):
         return self.object.package.can_user_manage_deprecation(self.request.user)
 
-    @property
+    @cached_property
     def can_manage_categories(self) -> bool:
         return check_validity(
             lambda: self.object.ensure_update_categories_permission(self.request.user)
@@ -514,12 +514,17 @@ def can_undeprecate(self):
     def can_unlist(self):
         return self.request.user.is_superuser
 
+    @cached_property
+    def can_moderate(self) -> bool:
+        return self.object.community.can_user_manage_packages(self.request.user)
+
     def get_review_panel(self):
-        if not self.object.community.can_user_manage_packages(self.request.user):
+        if not self.can_moderate:
             return None
         return {
             "reviewStatus": self.object.review_status,
             "rejectionReason": self.object.rejection_reason,
+            "internalNotes": self.object.notes,
             "packageListingId": self.object.pk,
         }
 
@@ -546,6 +551,8 @@ def get_context_data(self, *args, **kwargs):
         context["show_package_admin_link"] = can_view_package_admin(
             self.request.user, package_listing.package
         )
+        context["show_review_status"] = self.can_manage
+        context["show_internal_notes"] = self.can_moderate
 
         def format_category(cat: PackageCategory):
             return {"name": cat.name, "slug": cat.slug}