diff --git a/api/README.md b/api/README.md index db21ab6c..e5996008 100644 --- a/api/README.md +++ b/api/README.md @@ -385,6 +385,8 @@ The Issue endpoint supports the most comprehensive filtering options: - `upc` - UPC code - `page_count` - Number of pages - `rating` - Content rating object + - `average_rating` - Average community star rating (1.0 to 5.0, null if no ratings). Not included in the list endpoint. + - `rating_count` - Total number of community star ratings. Not included in the list endpoint. - `arcs` - Story arcs - `characters` - Characters appearing - `teams` - Teams appearing diff --git a/api/v1_0/serializers/issue.py b/api/v1_0/serializers/issue.py index 51f4e3e5..ceb4c76a 100644 --- a/api/v1_0/serializers/issue.py +++ b/api/v1_0/serializers/issue.py @@ -292,6 +292,10 @@ class IssueReadSerializer(serializers.ModelSerializer): series = IssueSeriesSerializer(read_only=True) reprints = ReprintSerializer(many=True, read_only=True) rating = RatingSerializer(read_only=True) + average_rating = serializers.DecimalField( + max_digits=3, decimal_places=2, coerce_to_string=False, read_only=True + ) + rating_count = serializers.IntegerField(read_only=True) resource_url = serializers.SerializerMethodField("get_resource_url") def get_resource_url(self, obj: Issue) -> str: @@ -327,6 +331,8 @@ class Meta: "desc", "image", "cover_hash", + "average_rating", + "rating_count", "arcs", "credits", "characters", diff --git a/api/views.py b/api/views.py index fb366a8f..aa8ab151 100644 --- a/api/views.py +++ b/api/views.py @@ -1,5 +1,16 @@ from django.db import models -from django.db.models import Avg, Case, Count, F, IntegerField, Prefetch, Q, Sum, When +from django.db.models import ( + Avg, + Case, + Count, + DecimalField, + F, + IntegerField, + Prefetch, + Q, + Sum, + When, +) from django.http import Http404 from django.shortcuts import get_object_or_404 from django.utils import timezone @@ -356,30 +367,39 @@ class IssueViewSet( def get_queryset(self): if self.action == "list": return Issue.objects.select_related("series", "series__series_type") - return Issue.objects.select_related( - "series", - "series__series_type", - "series__publisher", - "series__imprint", - "rating", - ).prefetch_related( - "series__genres", - "arcs", - "characters", - "teams", - "universes", - "variants", - Prefetch( - "credits_set", - queryset=Credits.objects.order_by("creator__name") - .distinct("creator__name") - .select_related("creator") - .prefetch_related("role"), - ), - Prefetch( - "reprints", - queryset=Issue.objects.select_related("series", "series__series_type"), - ), + return ( + Issue.objects.select_related( + "series", + "series__series_type", + "series__publisher", + "series__imprint", + "rating", + ) + .prefetch_related( + "series__genres", + "arcs", + "characters", + "teams", + "universes", + "variants", + Prefetch( + "credits_set", + queryset=Credits.objects.order_by("creator__name") + .distinct("creator__name") + .select_related("creator") + .prefetch_related("role"), + ), + Prefetch( + "reprints", + queryset=Issue.objects.select_related("series", "series__series_type"), + ), + ) + .annotate( + average_rating=Avg( + "ratings__rating", output_field=DecimalField(max_digits=3, decimal_places=2) + ), + rating_count=Count("ratings", distinct=True), + ) ) def get_serializer_class(self): diff --git a/comicsdb/models/common.py b/comicsdb/models/common.py index 691160e1..318f2c59 100644 --- a/comicsdb/models/common.py +++ b/comicsdb/models/common.py @@ -4,6 +4,10 @@ from django.db.models.functions import Now from django.utils.text import slugify +MIN_RATING = 1 +MAX_RATING = 5 +RATING_CHOICES = [(i, str(i)) for i in range(MIN_RATING, MAX_RATING + 1)] + def generate_slug_from_name(instance): base_slug = ( @@ -44,3 +48,17 @@ class CommonInfo(models.Model): class Meta: abstract = True + + +class AbstractRating(models.Model): + """Abstract base for a user's 1-5 star rating of a related object.""" + + rating = models.PositiveSmallIntegerField( + choices=RATING_CHOICES, + help_text="Star rating (1-5)", + ) + created_on = models.DateTimeField(db_default=Now()) + modified = models.DateTimeField(auto_now=True) + + class Meta: + abstract = True diff --git a/comicsdb/templates/comicsdb/issue_detail.html b/comicsdb/templates/comicsdb/issue_detail.html index e5d207bc..db9aac3a 100644 --- a/comicsdb/templates/comicsdb/issue_detail.html +++ b/comicsdb/templates/comicsdb/issue_detail.html @@ -64,6 +64,7 @@

{{ issue }}

{{ issue.rating.name }} {% endif %} + {% include "partials/rating_summary.html" with rated_object=issue average_rating=average_rating rating_count=rating_count %}

{# Navigation Bar #} @@ -628,6 +629,8 @@

Story Arc{{ arcs|pluralize }}

{% endif %} {% endwith %} +
+ {% include "partials/rating_widget.html" with rated_object=issue rate_url_name="issue-ratings:rate" rate_url_arg=issue.pk user_rating=user_rating average_rating=average_rating rating_count=rating_count show_ratings=True can_rate=user.is_authenticated %}

Identification Numbers

diff --git a/comicsdb/views/issue.py b/comicsdb/views/issue.py index 96de4f27..1efd3541 100644 --- a/comicsdb/views/issue.py +++ b/comicsdb/views/issue.py @@ -6,7 +6,7 @@ from django.contrib.auth.mixins import LoginRequiredMixin, PermissionRequiredMixin from django.core.exceptions import ObjectDoesNotExist from django.db import IntegrityError, transaction -from django.db.models import Prefetch +from django.db.models import Avg, Count, DecimalField, Prefetch from django.http import HttpResponse, HttpResponseRedirect from django.shortcuts import get_object_or_404 from django.template.loader import render_to_string @@ -32,6 +32,7 @@ build_active_filters, ) from comicsdb.views.mixins import LazyLoadMixin, SlugRedirectView +from issue_ratings.models import IssueRating from wish_list.models import WishListItem TOTAL_WEEKS_YEAR = 52 @@ -67,7 +68,9 @@ def get_context_data(self, **kwargs: Any) -> dict[str, Any]: class IssueDetail(DetailView): model = Issue queryset = ( - Issue.objects.select_related("series", "series__publisher", "series__series_type", "rating") + Issue.objects.select_related( + "series", "series__publisher", "series__series_type", "rating", "edited_by" + ) .defer( "created_on", "cover_hash", @@ -122,17 +125,31 @@ class IssueDetail(DetailView): ) ) + def get_queryset(self): + return ( + super() + .get_queryset() + .annotate( + average_rating=Avg( + "ratings__rating", output_field=DecimalField(max_digits=3, decimal_places=2) + ), + rating_count=Count("ratings", distinct=True), + ) + ) + def get_context_data(self, **kwargs): context = super().get_context_data(**kwargs) issue = context["object"] try: next_issue = issue.get_next_by_cover_date(series=issue.series) + next_issue.series = issue.series except ObjectDoesNotExist: next_issue = None try: previous_issue = issue.get_previous_by_cover_date(series=issue.series) + previous_issue.series = issue.series except ObjectDoesNotExist: previous_issue = None @@ -189,6 +206,13 @@ def get_context_data(self, **kwargs): wish_list__user=self.request.user, issue=issue, ).exists() + context["user_rating"] = IssueRating.objects.filter( + issue=issue, + user=self.request.user, + ).first() + + context["average_rating"] = issue.average_rating + context["rating_count"] = issue.rating_count return context diff --git a/comicsdb/views/ratings.py b/comicsdb/views/ratings.py new file mode 100644 index 00000000..b61fd304 --- /dev/null +++ b/comicsdb/views/ratings.py @@ -0,0 +1,38 @@ +""" +Shared helpers for HTMX star-rating update views. +""" + +from comicsdb.models.common import MAX_RATING, MIN_RATING + + +def parse_rating_action(raw_value): + """ + Parse a raw HTMX POST 'rating' value into an action. + + Returns ("set", rating) for a value in [MIN_RATING, MAX_RATING], + ("clear", None) for the 0 sentinel, or None if the input is missing, + non-numeric, or otherwise out of range (caller should no-op). + """ + if not raw_value: + return None + try: + rating = int(raw_value) + except ValueError: + return None + if MIN_RATING <= rating <= MAX_RATING: + return ("set", rating) + if rating == 0: + return ("clear", None) + return None + + +def apply_rating_update(model, lookup, raw_value): + """Set/update or clear a `model` row identified by `lookup` from a raw POST value.""" + action = parse_rating_action(raw_value) + if action is None: + return + kind, rating = action + if kind == "set": + model.objects.update_or_create(**lookup, defaults={"rating": rating}) + else: + model.objects.filter(**lookup).delete() diff --git a/issue_ratings/__init__.py b/issue_ratings/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/issue_ratings/admin.py b/issue_ratings/admin.py new file mode 100644 index 00000000..8f224c4f --- /dev/null +++ b/issue_ratings/admin.py @@ -0,0 +1,12 @@ +from django.contrib import admin + +from issue_ratings.models import IssueRating + + +@admin.register(IssueRating) +class IssueRatingAdmin(admin.ModelAdmin): + list_display = ("issue", "user", "rating", "modified") + list_filter = ("rating", "created_on") + search_fields = ("issue__series__name", "user__username") + readonly_fields = ("created_on", "modified") + autocomplete_fields = ["issue", "user"] diff --git a/issue_ratings/apps.py b/issue_ratings/apps.py new file mode 100644 index 00000000..3b2f26c0 --- /dev/null +++ b/issue_ratings/apps.py @@ -0,0 +1,6 @@ +from django.apps import AppConfig + + +class IssueRatingsConfig(AppConfig): + default_auto_field = "django.db.models.BigAutoField" + name = "issue_ratings" diff --git a/issue_ratings/management/__init__.py b/issue_ratings/management/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/issue_ratings/management/commands/__init__.py b/issue_ratings/management/commands/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/issue_ratings/management/commands/backfill_issue_ratings.py b/issue_ratings/management/commands/backfill_issue_ratings.py new file mode 100644 index 00000000..ab730c7f --- /dev/null +++ b/issue_ratings/management/commands/backfill_issue_ratings.py @@ -0,0 +1,60 @@ +""" +One-time backfill: sync existing user_collection ratings into issue_ratings. + +Populates issue_ratings.IssueRating from CollectionItem.rating values that were +set before the two apps were kept in sync via user_collection.signals. +""" + +from django.core.management.base import BaseCommand +from django.db import transaction + +from issue_ratings.models import IssueRating +from user_collection.models import CollectionItem + + +class Command(BaseCommand): + help = "Backfill IssueRating rows from existing CollectionItem ratings." + + def add_arguments(self, parser) -> None: + parser.add_argument( + "--dry-run", + action="store_true", + help="Report what would change without writing to the database", + ) + + def handle(self, *args, **options) -> None: + dry_run = options["dry_run"] + rated_items = CollectionItem.objects.filter(rating__isnull=False).select_related( + "issue", "user" + ) + + created_count = 0 + updated_count = 0 + unchanged_count = 0 + + with transaction.atomic(): + for item in rated_items.iterator(): + existing = IssueRating.objects.filter(issue=item.issue, user=item.user).first() + + if existing is None: + created_count += 1 + IssueRating.objects.create(issue=item.issue, user=item.user, rating=item.rating) + elif existing.rating != item.rating: + updated_count += 1 + existing.rating = item.rating + existing.save(update_fields=["rating"]) + else: + unchanged_count += 1 + + if dry_run: + transaction.set_rollback(True) + + verb = "Would sync" if dry_run else "Synced" + total = created_count + updated_count + unchanged_count + self.stdout.write( + self.style.SUCCESS( + f"{verb} {total} rated collection item(s): " + f"{created_count} to create, {updated_count} to update, " + f"{unchanged_count} already in sync." + ) + ) diff --git a/issue_ratings/migrations/0001_initial.py b/issue_ratings/migrations/0001_initial.py new file mode 100644 index 00000000..ba72161f --- /dev/null +++ b/issue_ratings/migrations/0001_initial.py @@ -0,0 +1,63 @@ +# Generated by Django 6.0.7 on 2026-07-13 15:56 + +import django.db.models.deletion +import django.db.models.functions.datetime +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + initial = True + + dependencies = [ + ("comicsdb", "0052_alter_historicalissue_price_currency_and_more"), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.CreateModel( + name="IssueRating", + fields=[ + ( + "id", + models.BigAutoField( + auto_created=True, primary_key=True, serialize=False, verbose_name="ID" + ), + ), + ( + "rating", + models.PositiveSmallIntegerField( + choices=[(1, "1"), (2, "2"), (3, "3"), (4, "4"), (5, "5")], + help_text="Star rating (1-5) for this issue", + ), + ), + ( + "created_on", + models.DateTimeField(db_default=django.db.models.functions.datetime.Now()), + ), + ("modified", models.DateTimeField(auto_now=True)), + ( + "issue", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="ratings", + to="comicsdb.issue", + ), + ), + ( + "user", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="issue_ratings", + to=settings.AUTH_USER_MODEL, + ), + ), + ], + options={ + "indexes": [ + models.Index(fields=["issue", "user"], name="issue_ratin_issue_i_f20318_idx") + ], + "unique_together": {("issue", "user")}, + }, + ), + ] diff --git a/issue_ratings/migrations/0002_alter_issuerating_rating.py b/issue_ratings/migrations/0002_alter_issuerating_rating.py new file mode 100644 index 00000000..44c9d7bd --- /dev/null +++ b/issue_ratings/migrations/0002_alter_issuerating_rating.py @@ -0,0 +1,20 @@ +# Generated by Django 6.0.7 on 2026-07-13 16:45 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("issue_ratings", "0001_initial"), + ] + + operations = [ + migrations.AlterField( + model_name="issuerating", + name="rating", + field=models.PositiveSmallIntegerField( + choices=[(1, "1"), (2, "2"), (3, "3"), (4, "4"), (5, "5")], + help_text="Star rating (1-5)", + ), + ), + ] diff --git a/issue_ratings/migrations/__init__.py b/issue_ratings/migrations/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/issue_ratings/models.py b/issue_ratings/models.py new file mode 100644 index 00000000..ab6f010e --- /dev/null +++ b/issue_ratings/models.py @@ -0,0 +1,29 @@ +from django.db import models + +from comicsdb.models.common import AbstractRating +from comicsdb.models.issue import Issue +from users.models import CustomUser + + +class IssueRating(AbstractRating): + """User ratings for issues.""" + + issue = models.ForeignKey( + Issue, + on_delete=models.CASCADE, + related_name="ratings", + ) + user = models.ForeignKey( + CustomUser, + on_delete=models.CASCADE, + related_name="issue_ratings", + ) + + class Meta: + unique_together = ["issue", "user"] + indexes = [ + models.Index(fields=["issue", "user"]), + ] + + def __str__(self) -> str: + return f"{self.user.username} - {self.issue}: {self.rating}" diff --git a/issue_ratings/urls.py b/issue_ratings/urls.py new file mode 100644 index 00000000..f7c3e9b8 --- /dev/null +++ b/issue_ratings/urls.py @@ -0,0 +1,9 @@ +from django.urls import path + +from issue_ratings.views import update_issue_rating + +app_name = "issue-ratings" + +urlpatterns = [ + path("/rate/", update_issue_rating, name="rate"), +] diff --git a/issue_ratings/views.py b/issue_ratings/views.py new file mode 100644 index 00000000..1ea826b9 --- /dev/null +++ b/issue_ratings/views.py @@ -0,0 +1,63 @@ +from django.contrib.auth.decorators import login_required +from django.db.models import Avg, Count +from django.http import HttpResponse +from django.shortcuts import get_object_or_404 +from django.template.loader import render_to_string +from django.views.decorators.http import require_POST + +from comicsdb.models.issue import Issue +from comicsdb.views.ratings import apply_rating_update +from issue_ratings.models import IssueRating + + +@login_required +@require_POST +def update_issue_rating(request, pk): + """HTMX view to update the rating of an issue.""" + issue = get_object_or_404(Issue, pk=pk) + + apply_rating_update( + IssueRating, + {"issue": issue, "user": request.user}, + request.POST.get("rating"), + ) + + # Get user's current rating and average + user_rating = IssueRating.objects.filter( + issue=issue, + user=request.user, + ).first() + + # Calculate average rating + avg_data = issue.ratings.aggregate( + avg=Avg("rating"), + count=Count("id"), + ) + + # Return the updated rating widget, plus an out-of-band update for the + # average-rating summary shown in the page header so the two stay in sync. + widget_html = render_to_string( + "partials/rating_widget.html", + { + "rated_object": issue, + "rate_url_name": "issue-ratings:rate", + "rate_url_arg": issue.pk, + "user_rating": user_rating, + "average_rating": avg_data["avg"], + "rating_count": avg_data["count"], + "show_ratings": True, + "can_rate": True, + }, + request=request, + ) + summary_html = render_to_string( + "partials/rating_summary.html", + { + "rated_object": issue, + "average_rating": avg_data["avg"], + "rating_count": avg_data["count"], + "oob": True, + }, + request=request, + ) + return HttpResponse(widget_html + summary_html) diff --git a/metron/settings.py b/metron/settings.py index 3d35da04..c4e0bb51 100644 --- a/metron/settings.py +++ b/metron/settings.py @@ -78,6 +78,7 @@ "user_collection", "pull_list", "wish_list", + "issue_ratings", "polls", "timeline", ] diff --git a/metron/urls.py b/metron/urls.py index 8fcf6f70..5d33efd4 100644 --- a/metron/urls.py +++ b/metron/urls.py @@ -25,6 +25,7 @@ team as team_urls, universe as universe_urls, ) +from issue_ratings import urls as issue_ratings_urls from polls import urls as polls_urls from pull_list import urls as pull_list_urls from reading_lists import urls as reading_lists_urls @@ -59,6 +60,7 @@ path("", include(home_urls)), path("imprint/", include(imprint_urls)), path("issue/", include(issue_urls)), + path("issue-ratings/", include(issue_ratings_urls)), path("jsi18n/", JavaScriptCatalog.as_view(), name="javascript-catalog"), path("publisher/", include(publisher_urls)), path("polls/", include(polls_urls)), diff --git a/pyproject.toml b/pyproject.toml index 20bb7232..87102c23 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -99,7 +99,9 @@ combine-as-imports = true "comicsdb/models/__init__.py" = ["I001"] "comicsdb/migrations/*.py" = ["E501", "N806"] "django_nyt/migrations/*.py" = ["E501", "N806"] +"issue_ratings/migrations/*.py" = ["E501", "N806"] "pull_list/migrations/*.py" = ["E501", "N806"] +"reading_lists/migrations/*.py" = ["E501", "N806"] "tests/*/*.py" = ["PLR2004", "PT004", "S101", "PLR0913"] "users/apps.py" = ["PLC0415",] "users/migrations/*.py" = ["E501", "N806"] diff --git a/reading_lists/TECHNICAL.md b/reading_lists/TECHNICAL.md index 54486703..eeff1b9e 100644 --- a/reading_lists/TECHNICAL.md +++ b/reading_lists/TECHNICAL.md @@ -639,12 +639,14 @@ elif rating == 0: **Response:** -Returns the `reading_list_rating.html` partial template with context: +Returns the shared `partials/rating_widget.html` partial template (also used by `issue_ratings`) with context: -- `reading_list`: The rated reading list +- `rated_object`: The rated reading list +- `rate_url_name` / `rate_url_arg`: URL name and argument used to build the rate/clear endpoint - `user_rating`: User's updated rating object - `average_rating`: Recalculated average rating - `rating_count`: Updated rating count +- `show_ratings` / `can_rate`: Always `True` here, since the view already rejected private lists and self-rating before reaching this response **URL:** `/reading-lists//rate/` @@ -1202,7 +1204,7 @@ The web interface provides a dropdown with: | `remove_issue_confirm.html` | Remove issue confirmation | Issue details | | `partials/readinglist_card.html` | List card (grid item) | Cover thumbnail, list type tag, year range, rating stars, privacy badge, truncated description | | `partials/readinglist_filter.html` | Filter/search panel | Quick search + collapsible advanced filters, active-filter tag | -| `partials/reading_list_rating.html` | Rating component | HTMX-powered star rating, average display | +| `partials/rating_widget.html` (shared, in top-level `templates/partials/`) | Rating component | HTMX-powered star rating, average display; also used by `issue_ratings` | | `partials/readinglist_item.html` | Single item display | Issue type badge, inline edit button, remove button | | `partials/readinglist_item_edit.html` | Single item edit form | Issue type dropdown, save/cancel buttons | | `partials/readinglist_item_list.html` | Item list + load-more button | Rendered by `ReadingListItemsLoadMore`; re-renders itself and the load-more button via an out-of-band (`hx-swap-oob`) swap | @@ -1253,16 +1255,16 @@ htmx.onLoad(function() { ### Rating Partial Template -**File:** `partials/reading_list_rating.html` +**File:** `partials/rating_widget.html` (shared with `issue_ratings`, lives in the top-level `templates/partials/` directory) The rating system uses HTMX for instant, no-reload rating updates: ```html diff --git a/reading_lists/migrations/0011_alter_readinglistrating_rating.py b/reading_lists/migrations/0011_alter_readinglistrating_rating.py new file mode 100644 index 00000000..749c3e5a --- /dev/null +++ b/reading_lists/migrations/0011_alter_readinglistrating_rating.py @@ -0,0 +1,20 @@ +# Generated by Django 6.0.7 on 2026-07-13 16:45 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("reading_lists", "0010_add_creator_list_type"), + ] + + operations = [ + migrations.AlterField( + model_name="readinglistrating", + name="rating", + field=models.PositiveSmallIntegerField( + choices=[(1, "1"), (2, "2"), (3, "3"), (4, "4"), (5, "5")], + help_text="Star rating (1-5)", + ), + ), + ] diff --git a/reading_lists/models.py b/reading_lists/models.py index 1bb059ab..4e7a2f05 100644 --- a/reading_lists/models.py +++ b/reading_lists/models.py @@ -4,7 +4,7 @@ from django.urls import reverse from sorl.thumbnail import ImageField -from comicsdb.models.common import CommonInfo, pre_save_slug +from comicsdb.models.common import AbstractRating, CommonInfo, pre_save_slug from comicsdb.models.issue import Issue from comicsdb.models.publisher import Publisher from users.models import CustomUser @@ -192,7 +192,7 @@ def __str__(self) -> str: return f"{self.reading_list.name} - {self.issue} (Order: {self.order})" -class ReadingListRating(models.Model): +class ReadingListRating(AbstractRating): """User ratings for reading lists.""" reading_list = models.ForeignKey( @@ -205,12 +205,6 @@ class ReadingListRating(models.Model): on_delete=models.CASCADE, related_name="reading_list_ratings", ) - rating = models.PositiveSmallIntegerField( - choices=[(i, str(i)) for i in range(1, 6)], - help_text="Star rating (1-5) for this reading list", - ) - created_on = models.DateTimeField(db_default=models.functions.Now()) - modified = models.DateTimeField(auto_now=True) class Meta: unique_together = ["reading_list", "user"] diff --git a/reading_lists/templates/reading_lists/partials/reading_list_rating.html b/reading_lists/templates/reading_lists/partials/reading_list_rating.html deleted file mode 100644 index 6012c57e..00000000 --- a/reading_lists/templates/reading_lists/partials/reading_list_rating.html +++ /dev/null @@ -1,104 +0,0 @@ -
- {% if not reading_list.is_private %} - {# Display average rating #} -
- Average Rating: - {% if average_rating %} - - {% for star_num in "12345" %} - - {% endfor %} - - {{ average_rating|floatformat:1 }} ({{ rating_count }} rating{{ rating_count|pluralize }}) - {% else %} - No ratings yet - {% endif %} -
- {# User's rating (if authenticated and not the owner) #} - {% if user.is_authenticated and reading_list.user != user %} -
- Your Rating: -
- {% for star_num in "12345" %} - - {% endfor %} - {% if user_rating %} - - {% endif %} -
-
- {% endif %} - {% endif %} -
- diff --git a/reading_lists/templates/reading_lists/readinglist_detail.html b/reading_lists/templates/reading_lists/readinglist_detail.html index 2062d2b1..d10a92b3 100644 --- a/reading_lists/templates/reading_lists/readinglist_detail.html +++ b/reading_lists/templates/reading_lists/readinglist_detail.html @@ -272,7 +272,7 @@

List details

{{ reading_list.modified|date:"F d, Y" }}
- {% include "reading_lists/partials/reading_list_rating.html" %} + {% include "partials/rating_widget.html" with rated_object=reading_list rate_url_name="reading-list:rate" rate_url_arg=reading_list.slug user_rating=user_rating average_rating=average_rating rating_count=rating_count show_ratings=show_ratings can_rate=can_rate %}
{% if series_breakdown %} diff --git a/reading_lists/views.py b/reading_lists/views.py index 999a0d80..6c049f7d 100644 --- a/reading_lists/views.py +++ b/reading_lists/views.py @@ -18,6 +18,7 @@ from comicsdb.models.credits import Credits, Role from comicsdb.models.issue import Issue from comicsdb.views.mixins import LazyLoadMixin, SearchMixin +from comicsdb.views.ratings import apply_rating_update from reading_lists.forms import ( AddIssuesFromArcForm, AddIssuesFromSeriesForm, @@ -35,10 +36,6 @@ # Pagination constant for reading list detail view READING_LIST_DETAIL_PAGINATE_BY = 50 -# Rating constants -MIN_RATING = 1 -MAX_RATING = 5 - _NON_FILTER_PARAMS = {"page"} _FILTER_LABELS = { @@ -463,6 +460,10 @@ def get_context_data(self, **kwargs): context["user_rating"] = user_rating context["average_rating"] = reading_list.average_rating context["rating_count"] = reading_list.rating_count + context["show_ratings"] = not reading_list.is_private + context["can_rate"] = ( + self.request.user.is_authenticated and reading_list.user != self.request.user + ) # Add annotated year data to context context["start_year"] = reading_list.start_year_annotated @@ -849,24 +850,11 @@ def update_reading_list_rating(request, slug): if reading_list.user == request.user: return HttpResponseForbidden("Cannot rate your own reading list") - rating_value = request.POST.get("rating") - if rating_value: - try: - rating = int(rating_value) - if MIN_RATING <= rating <= MAX_RATING: - # Update or create rating - ReadingListRating.objects.update_or_create( - reading_list=reading_list, - user=request.user, - defaults={"rating": rating}, - ) - elif rating == 0: # Allow clearing the rating - ReadingListRating.objects.filter( - reading_list=reading_list, - user=request.user, - ).delete() - except ValueError: - pass + apply_rating_update( + ReadingListRating, + {"reading_list": reading_list, "user": request.user}, + request.POST.get("rating"), + ) # Get user's current rating and average user_rating = ReadingListRating.objects.filter( @@ -883,12 +871,16 @@ def update_reading_list_rating(request, slug): # Return the updated rating partial return render( request, - "reading_lists/partials/reading_list_rating.html", + "partials/rating_widget.html", { - "reading_list": reading_list, + "rated_object": reading_list, + "rate_url_name": "reading-list:rate", + "rate_url_arg": reading_list.slug, "user_rating": user_rating, "average_rating": avg_data["avg"], "rating_count": avg_data["count"], + "show_ratings": True, + "can_rate": True, }, ) diff --git a/templates/partials/rating_summary.html b/templates/partials/rating_summary.html new file mode 100644 index 00000000..03d83b23 --- /dev/null +++ b/templates/partials/rating_summary.html @@ -0,0 +1,25 @@ +{% comment %} +Compact average-rating summary (stars + count), used in page headers. + +Shares state with partials/rating_widget.html. Pass oob=True when rendering +from a rating-update view so this refreshes via HTMX out-of-band swap +alongside the main rating widget, instead of going stale until reload. + +Required context: + rated_object - the instance being rated + average_rating - float average, or None + rating_count - int + oob - bool, optional; adds hx-swap-oob +{% endcomment %} + + {% if average_rating %} + · + + {% for star_num in "12345" %} + + {% endfor %} + + {{ average_rating|floatformat:1 }} ({{ rating_count }} rating{{ rating_count|pluralize }}) + {% endif %} + diff --git a/templates/partials/rating_widget.html b/templates/partials/rating_widget.html new file mode 100644 index 00000000..a9bcfaa6 --- /dev/null +++ b/templates/partials/rating_widget.html @@ -0,0 +1,115 @@ +{% comment %} +Generic 1-5 star rating widget, shared by reading_lists and issue_ratings. + +Required context: + rated_object - the instance being rated + rate_url_name - namespaced view name, e.g. "reading-list:rate" or "issue-ratings:rate" + rate_url_arg - the identifier the rate URL expects (slug, pk, ...) + user_rating - the requesting user's rating instance, or None + average_rating - float average, or None + rating_count - int + show_ratings - bool; whole widget renders only if true + can_rate - bool; gates the interactive "Your Rating" stars +{% endcomment %} +{% if show_ratings %} +
+ {# Display average rating #} +
+ Average Rating: + {% if average_rating %} + + {% for star_num in "12345" %} + + {% endfor %} + + {{ average_rating|floatformat:1 }} ({{ rating_count }} rating{{ rating_count|pluralize }}) + {% else %} + No ratings yet + {% endif %} +
+ {# User's rating (if allowed) #} + {% if can_rate %} +
+ Your Rating: +
+ {% for star_num in "12345" %} + + {% endfor %} + {% if user_rating %} + + {% endif %} +
+
+ {% endif %} +
+ +{% endif %} diff --git a/tests/comicsdb/test_api_issue.py b/tests/comicsdb/test_api_issue.py index 4976ff2e..c478d5e3 100644 --- a/tests/comicsdb/test_api_issue.py +++ b/tests/comicsdb/test_api_issue.py @@ -15,6 +15,7 @@ from comicsdb.models.series import Series from comicsdb.models.team import Team from comicsdb.models.universe import Universe +from issue_ratings.models import IssueRating @pytest.fixture @@ -340,6 +341,39 @@ def test_filter_by_role_id_no_match(api_client_with_credentials, basic_issue: Is assert resp.data["count"] == 0 +def test_list_excludes_rating_fields(api_client_with_credentials, list_of_issues): + """The list endpoint is high-traffic; rating fields are only exposed on detail.""" + resp = api_client_with_credentials.get(reverse("api:issue-list")) + assert resp.status_code == status.HTTP_200_OK + assert "average_rating" not in resp.data["results"][0] + assert "rating_count" not in resp.data["results"][0] + + +def test_detail_rating_fields_no_ratings(api_client_with_credentials, issue_with_arc: Issue): + resp = api_client_with_credentials.get( + reverse("api:issue-detail", kwargs={"pk": issue_with_arc.pk}) + ) + assert resp.status_code == status.HTTP_200_OK + assert resp.data["average_rating"] is None + assert resp.data["rating_count"] == 0 + + +def test_detail_rating_fields_with_ratings( + api_client_with_credentials, issue_with_arc: Issue, create_user +): + user1 = create_user(username="api_rater_1") + user2 = create_user(username="api_rater_2") + IssueRating.objects.create(issue=issue_with_arc, user=user1, rating=5) + IssueRating.objects.create(issue=issue_with_arc, user=user2, rating=3) + + resp = api_client_with_credentials.get( + reverse("api:issue-detail", kwargs={"pk": issue_with_arc.pk}) + ) + assert resp.status_code == status.HTTP_200_OK + assert resp.data["average_rating"] == 4.0 + assert resp.data["rating_count"] == 2 + + def test_filter_by_multiple_role_ids( api_client_with_credentials, basic_issue: Issue, john_byrne: Creator, writer: Role ): diff --git a/tests/comicsdb/test_issue_views.py b/tests/comicsdb/test_issue_views.py index 99b19ace..80291b79 100644 --- a/tests/comicsdb/test_issue_views.py +++ b/tests/comicsdb/test_issue_views.py @@ -10,6 +10,7 @@ from comicsdb.models.issue import Issue from comicsdb.models.publisher import Publisher from comicsdb.models.series import Series, SeriesType +from issue_ratings.models import IssueRating from wish_list.models import WishList, WishListItem HTML_OK_CODE = 200 @@ -48,6 +49,28 @@ def test_issue_detail_on_wish_list_true_when_on_list(basic_issue, auto_login_use assert resp.context["on_wish_list"] is True +def test_issue_detail_rating_context_no_ratings(basic_issue, auto_login_user): + client, _ = auto_login_user() + resp = client.get(f"/issue/{basic_issue.slug}/") + assert resp.status_code == HTML_OK_CODE + assert resp.context["average_rating"] is None + assert resp.context["rating_count"] == 0 + assert resp.context["user_rating"] is None + + +def test_issue_detail_rating_context_with_ratings(basic_issue, auto_login_user, create_user): + client, user = auto_login_user() + other_user = create_user(username="other_rater") + IssueRating.objects.create(issue=basic_issue, user=user, rating=4) + IssueRating.objects.create(issue=basic_issue, user=other_user, rating=2) + + resp = client.get(f"/issue/{basic_issue.slug}/") + assert resp.status_code == HTML_OK_CODE + assert resp.context["average_rating"] == 3.0 + assert resp.context["rating_count"] == 2 + assert resp.context["user_rating"].rating == 4 + + # Issue Search def test_issue_search_view_url_exists_at_desired_location(auto_login_user): client, _ = auto_login_user() diff --git a/tests/issue_ratings/__init__.py b/tests/issue_ratings/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/issue_ratings/conftest.py b/tests/issue_ratings/conftest.py new file mode 100644 index 00000000..b74430ac --- /dev/null +++ b/tests/issue_ratings/conftest.py @@ -0,0 +1,61 @@ +"""Fixtures for issue_ratings app tests.""" + +from datetime import date + +import pytest + +from comicsdb.models.issue import Issue +from comicsdb.models.publisher import Publisher +from comicsdb.models.series import Series + + +@pytest.fixture +def rating_user(db, create_user): + """Create a user for issue rating tests.""" + return create_user(username="rating_user") + + +@pytest.fixture +def other_rating_user(db, create_user): + """Create another user for issue rating tests.""" + return create_user(username="other_rating_user") + + +@pytest.fixture +def rating_publisher(create_user): + """Create a publisher for issue rating tests.""" + user = create_user() + return Publisher.objects.create( + name="Rating Test Publisher", slug="rating-test-publisher", edited_by=user, created_by=user + ) + + +@pytest.fixture +def rating_series(create_user, rating_publisher, single_issue_type): + """Create a series for issue rating tests.""" + user = create_user() + return Series.objects.create( + name="Rating Test Series", + slug="rating-test-series", + publisher=rating_publisher, + volume="1", + year_began=2020, + series_type=single_issue_type, + status=Series.Status.ONGOING, + edited_by=user, + created_by=user, + ) + + +@pytest.fixture +def rating_issue(create_user, rating_series): + """Create an issue for issue rating tests.""" + user = create_user() + return Issue.objects.create( + series=rating_series, + number="1", + slug="rating-test-series-1", + cover_date=date(2020, 1, 1), + edited_by=user, + created_by=user, + ) diff --git a/tests/issue_ratings/test_backfill_command.py b/tests/issue_ratings/test_backfill_command.py new file mode 100644 index 00000000..055aee93 --- /dev/null +++ b/tests/issue_ratings/test_backfill_command.py @@ -0,0 +1,95 @@ +"""Tests for the backfill_issue_ratings management command.""" + +import io + +from django.core.management import call_command +from django.db.models.signals import post_save + +from issue_ratings.models import IssueRating +from user_collection.models import CollectionItem +from user_collection.signals import sync_issue_rating_from_collection_item + + +def create_legacy_rated_item(**kwargs): + """Create a CollectionItem with a rating without the sync signal firing. + + Simulates data that predates the user_collection -> issue_ratings sync feature. + """ + post_save.disconnect( + sync_issue_rating_from_collection_item, + sender=CollectionItem, + dispatch_uid="post_save_sync_issue_rating_from_collection_item", + ) + try: + return CollectionItem.objects.create(**kwargs) + finally: + post_save.connect( + sync_issue_rating_from_collection_item, + sender=CollectionItem, + dispatch_uid="post_save_sync_issue_rating_from_collection_item", + ) + + +def run_backfill(**options): + out = io.StringIO() + call_command("backfill_issue_ratings", stdout=out, **options) + return out.getvalue() + + +def test_backfill_creates_missing_issue_ratings(rating_issue, rating_user): + item = create_legacy_rated_item(issue=rating_issue, user=rating_user, rating=4) + assert not IssueRating.objects.filter(issue=item.issue, user=item.user).exists() + + output = run_backfill() + + rating = IssueRating.objects.get(issue=item.issue, user=item.user) + assert rating.rating == 4 + assert "1 to create" in output + + +def test_backfill_updates_conflicting_issue_ratings(rating_issue, rating_user): + item = create_legacy_rated_item(issue=rating_issue, user=rating_user, rating=5) + IssueRating.objects.create(issue=item.issue, user=item.user, rating=2) + + output = run_backfill() + + rating = IssueRating.objects.get(issue=item.issue, user=item.user) + assert rating.rating == 5 + assert "1 to update" in output + + +def test_backfill_leaves_matching_ratings_unchanged(rating_issue, rating_user): + item = create_legacy_rated_item(issue=rating_issue, user=rating_user, rating=3) + IssueRating.objects.create(issue=item.issue, user=item.user, rating=3) + + output = run_backfill() + + assert "1 already in sync" in output + + +def test_backfill_ignores_collection_items_without_rating(rating_issue, rating_user): + create_legacy_rated_item(issue=rating_issue, user=rating_user, rating=None) + + output = run_backfill() + + assert not IssueRating.objects.filter(issue=rating_issue, user=rating_user).exists() + assert "0 rated collection item(s)" in output + + +def test_backfill_dry_run_makes_no_changes(rating_issue, rating_user): + item = create_legacy_rated_item(issue=rating_issue, user=rating_user, rating=4) + + output = run_backfill(dry_run=True) + + assert not IssueRating.objects.filter(issue=item.issue, user=item.user).exists() + assert "Would sync" in output + assert "1 to create" in output + + +def test_backfill_is_idempotent(rating_issue, rating_user): + create_legacy_rated_item(issue=rating_issue, user=rating_user, rating=4) + + run_backfill() + second_run_output = run_backfill() + + assert "0 to create, 0 to update" in second_run_output diff --git a/tests/issue_ratings/test_views.py b/tests/issue_ratings/test_views.py new file mode 100644 index 00000000..0895d619 --- /dev/null +++ b/tests/issue_ratings/test_views.py @@ -0,0 +1,86 @@ +"""Tests for issue_ratings views.""" + +from django.db.models import Avg +from django.urls import reverse + +from issue_ratings.models import IssueRating + +HTTP_200_OK = 200 +HTTP_302_FOUND = 302 + + +class TestIssueRating: + """Test cases for issue rating functionality.""" + + def test_update_rating_requires_login(self, client, rating_issue): + """Test that rating requires authentication.""" + url = reverse("issue-ratings:rate", kwargs={"pk": rating_issue.pk}) + resp = client.post(url, data={"rating": "5"}) + assert resp.status_code == HTTP_302_FOUND + assert "/login/" in resp.url + + def test_update_rating_set_rating(self, client, rating_issue, test_password, rating_user): + """Test setting a rating on an issue.""" + client.login(username=rating_user.username, password=test_password) + url = reverse("issue-ratings:rate", kwargs={"pk": rating_issue.pk}) + + resp = client.post(url, data={"rating": "5"}) + assert resp.status_code == HTTP_200_OK + + rating = IssueRating.objects.filter(issue=rating_issue, user=rating_user).first() + assert rating is not None + assert rating.rating == 5 + + def test_update_rating_updates_existing(self, client, rating_issue, test_password, rating_user): + """Test that rating twice updates rather than duplicates the row.""" + client.login(username=rating_user.username, password=test_password) + url = reverse("issue-ratings:rate", kwargs={"pk": rating_issue.pk}) + + client.post(url, data={"rating": "2"}) + client.post(url, data={"rating": "4"}) + + ratings = IssueRating.objects.filter(issue=rating_issue, user=rating_user) + assert ratings.count() == 1 + assert ratings.first().rating == 4 + + def test_update_rating_clear_rating(self, client, rating_issue, test_password, rating_user): + """Test clearing a rating.""" + IssueRating.objects.create(issue=rating_issue, user=rating_user, rating=5) + + client.login(username=rating_user.username, password=test_password) + url = reverse("issue-ratings:rate", kwargs={"pk": rating_issue.pk}) + + resp = client.post(url, data={"rating": "0"}) + assert resp.status_code == HTTP_200_OK + + assert not IssueRating.objects.filter(issue=rating_issue, user=rating_user).exists() + + def test_update_rating_invalid_values(self, client, rating_issue, test_password, rating_user): + """Test that invalid rating values are rejected.""" + client.login(username=rating_user.username, password=test_password) + url = reverse("issue-ratings:rate", kwargs={"pk": rating_issue.pk}) + + resp = client.post(url, data={"rating": "10"}) + assert resp.status_code == HTTP_200_OK + assert not IssueRating.objects.filter(issue=rating_issue, user=rating_user).exists() + + resp = client.post(url, data={"rating": "-1"}) + assert resp.status_code == HTTP_200_OK + assert not IssueRating.objects.filter(issue=rating_issue, user=rating_user).exists() + + resp = client.post(url, data={"rating": "not-a-number"}) + assert resp.status_code == HTTP_200_OK + assert not IssueRating.objects.filter(issue=rating_issue, user=rating_user).exists() + + def test_average_rating_calculation(self, rating_issue, create_user): + """Test that average rating is calculated correctly.""" + user1 = create_user(username="avg_user1") + user2 = create_user(username="avg_user2") + user3 = create_user(username="avg_user3") + + IssueRating.objects.create(issue=rating_issue, user=user1, rating=5) + IssueRating.objects.create(issue=rating_issue, user=user2, rating=3) + IssueRating.objects.create(issue=rating_issue, user=user3, rating=4) + + avg_rating = rating_issue.ratings.aggregate(avg=Avg("rating"))["avg"] + assert avg_rating == 4.0 diff --git a/tests/user_collection/test_collection_update.py b/tests/user_collection/test_collection_update.py index 5795b065..bb342c91 100644 --- a/tests/user_collection/test_collection_update.py +++ b/tests/user_collection/test_collection_update.py @@ -3,6 +3,7 @@ from django.urls import reverse from rest_framework import status +from issue_ratings.models import IssueRating from user_collection.models import CollectionItem, ReadDate @@ -30,6 +31,18 @@ def test_patch_updates_rating(api_client, collection_user, collection_item): assert collection_item.rating == 4 +def test_patch_rating_syncs_to_issue_rating(api_client, collection_user, collection_item): + """PATCHing a collection item's rating syncs the community IssueRating too.""" + api_client.force_authenticate(user=collection_user) + + api_client.patch( + reverse("api:collection-detail", args=[collection_item.id]), + {"rating": 4}, + ) + rating = IssueRating.objects.get(issue=collection_item.issue, user=collection_user) + assert rating.rating == 4 + + def test_patch_rating_creates_no_read_date_rows(api_client, collection_user, collection_item): """Updating rating via PATCH does not create any ReadDate rows.""" api_client.force_authenticate(user=collection_user) diff --git a/tests/user_collection/test_signals.py b/tests/user_collection/test_signals.py new file mode 100644 index 00000000..13a36912 --- /dev/null +++ b/tests/user_collection/test_signals.py @@ -0,0 +1,70 @@ +"""Tests for user_collection signals.""" + +from issue_ratings.models import IssueRating +from user_collection.models import CollectionItem + + +def test_setting_collection_rating_creates_issue_rating(collection_user, collection_item): + """Saving a CollectionItem with a rating creates a matching community IssueRating.""" + assert not IssueRating.objects.filter( + issue=collection_item.issue, user=collection_user + ).exists() + + collection_item.rating = 4 + collection_item.save(update_fields=["rating"]) + + rating = IssueRating.objects.get(issue=collection_item.issue, user=collection_user) + assert rating.rating == 4 + + +def test_updating_collection_rating_updates_issue_rating(collection_user, collection_item): + """Changing an existing collection rating updates the IssueRating rather than duplicating it.""" + collection_item.rating = 2 + collection_item.save(update_fields=["rating"]) + + collection_item.rating = 5 + collection_item.save(update_fields=["rating"]) + + ratings = IssueRating.objects.filter(issue=collection_item.issue, user=collection_user) + assert ratings.count() == 1 + assert ratings.first().rating == 5 + + +def test_clearing_collection_rating_deletes_issue_rating(collection_user, collection_item): + """Clearing a collection rating removes the corresponding IssueRating.""" + collection_item.rating = 3 + collection_item.save(update_fields=["rating"]) + assert IssueRating.objects.filter(issue=collection_item.issue, user=collection_user).exists() + + collection_item.rating = None + collection_item.save(update_fields=["rating"]) + + assert not IssueRating.objects.filter( + issue=collection_item.issue, user=collection_user + ).exists() + + +def test_sync_overwrites_independently_set_issue_rating(collection_user, collection_item): + """Setting a collection rating overwrites a pre-existing, independently-set community rating.""" + IssueRating.objects.create(issue=collection_item.issue, user=collection_user, rating=1) + + collection_item.rating = 5 + collection_item.save(update_fields=["rating"]) + + rating = IssueRating.objects.get(issue=collection_item.issue, user=collection_user) + assert rating.rating == 5 + + +def test_sync_scoped_to_issue_and_user(collection_user, other_collection_user, collection_issue_1): + """Syncing only ever touches the (issue, user) pair for the saved collection item.""" + CollectionItem.objects.create( + user=other_collection_user, + issue=collection_issue_1, + quantity=1, + rating=2, + ) + + ratings = IssueRating.objects.filter(issue=collection_issue_1) + assert ratings.count() == 1 + assert ratings.get().user == other_collection_user + assert ratings.get().rating == 2 diff --git a/tests/user_collection/test_views.py b/tests/user_collection/test_views.py index ac19dd8d..71a0121a 100644 --- a/tests/user_collection/test_views.py +++ b/tests/user_collection/test_views.py @@ -8,6 +8,7 @@ from comicsdb.models.issue import Issue from comicsdb.models.series import Series +from issue_ratings.models import IssueRating from user_collection.models import CollectionItem, ReadDate HTTP_200_OK = 200 @@ -1130,6 +1131,22 @@ def test_update_rating_set_rating( collection_item.refresh_from_db() assert collection_item.rating == 3 + def test_update_rating_syncs_to_issue_rating( + self, client, collection_user, collection_item, test_password + ): + """Rating a collection item through the HTMX endpoint syncs the community IssueRating.""" + client.login(username=collection_user.username, password=test_password) + url = reverse("user_collection:rate", kwargs={"pk": collection_item.pk}) + + client.post(url, {"rating": "4"}) + rating = IssueRating.objects.get(issue=collection_item.issue, user=collection_user) + assert rating.rating == 4 + + client.post(url, {"rating": "0"}) + assert not IssueRating.objects.filter( + issue=collection_item.issue, user=collection_user + ).exists() + def test_update_rating_all_valid_values( self, client, collection_user, collection_item, test_password ): diff --git a/user_collection/apps.py b/user_collection/apps.py index 7293a21f..b5fe26a8 100644 --- a/user_collection/apps.py +++ b/user_collection/apps.py @@ -1,6 +1,17 @@ from django.apps import AppConfig +from django.db.models.signals import post_save + +from user_collection.signals import sync_issue_rating_from_collection_item class UserCollectionConfig(AppConfig): default_auto_field = "django.db.models.BigAutoField" name = "user_collection" + + def ready(self): + collection_item = self.get_model("CollectionItem") + post_save.connect( + sync_issue_rating_from_collection_item, + sender=collection_item, + dispatch_uid="post_save_sync_issue_rating_from_collection_item", + ) diff --git a/user_collection/signals.py b/user_collection/signals.py new file mode 100644 index 00000000..5828e299 --- /dev/null +++ b/user_collection/signals.py @@ -0,0 +1,15 @@ +def sync_issue_rating_from_collection_item(sender, instance, **kwargs): + """Keep the community IssueRating in sync with a user's personal collection rating.""" + from issue_ratings.models import IssueRating # noqa: PLC0415 + + if instance.rating is not None: + IssueRating.objects.update_or_create( + issue_id=instance.issue_id, + user_id=instance.user_id, + defaults={"rating": instance.rating}, + ) + else: + IssueRating.objects.filter( + issue_id=instance.issue_id, + user_id=instance.user_id, + ).delete() diff --git a/user_collection/views.py b/user_collection/views.py index 793e16b7..4211c513 100644 --- a/user_collection/views.py +++ b/user_collection/views.py @@ -27,13 +27,10 @@ from comicsdb.models.issue import Issue from comicsdb.models.publisher import Publisher from comicsdb.models.series import Series, SeriesType +from comicsdb.views.ratings import parse_rating_action from user_collection.forms import AddIssuesFromSeriesForm, CollectionItemForm from user_collection.models import GRADE_CHOICES, CollectionItem, ReadDate -# Rating constants -MIN_RATING = 1 -MAX_RATING = 5 - class CollectionListView(LoginRequiredMixin, ListView): """Display the current user's collection items.""" @@ -481,18 +478,11 @@ def update_rating(request, pk): """HTMX view to update the rating of a collection item.""" item = get_object_or_404(CollectionItem, pk=pk, user=request.user) - rating_value = request.POST.get("rating") - if rating_value: - try: - rating = int(rating_value) - # Allow ratings from MIN_RATING to MAX_RATING - if MIN_RATING <= rating <= MAX_RATING: - item.rating = rating - elif rating == 0: # Allow clearing the rating - item.rating = None - item.save(update_fields=["rating"]) - except ValueError: - pass + action = parse_rating_action(request.POST.get("rating")) + if action is not None: + kind, rating = action + item.rating = rating if kind == "set" else None + item.save(update_fields=["rating"]) # Return the updated star rating partial return render(