Ratings: implement replies by maintainers #181

Merged
Oleg-Komarov merged 7 commits from rating-reply into main 2024-06-11 12:35:03 +02:00
16 changed files with 243 additions and 86 deletions

View File

@ -383,6 +383,9 @@
width: 100%
.rating-form
.box
margin-left: calc(var(--spacer) * 2.5)
select
color: var(--color-warning)
@ -391,6 +394,15 @@
&:focus
color: var(--color-warning)
textarea
min-height: calc(var(--spacer) * 6)
.rating-reply
@extend blockquote
+margin(3, top)
margin-bottom: 0 !important
// TODO: consider adding component boxed nav generic to web-assets, and make variants on top of that
.nav-pills
@extend .dropdown-menu

View File

@ -349,7 +349,6 @@ class Extension(CreatedModifiedMixin, TrackChangesMixin, models.Model):
return (
not self.has_maintainer(user)
and not self.ratings.filter(
reply_to=None,
user_id=user.pk,
).exists()
)
@ -401,13 +400,7 @@ class Extension(CreatedModifiedMixin, TrackChangesMixin, models.Model):
@property
def text_ratings_count(self) -> int:
return len(
[
r
for r in self.ratings.all()
if r.text is not None and r.is_listed and r.reply_to is None
]
)
return len([r for r in self.ratings.all() if r.text is not None and r.is_listed])
@property
def total_ratings_count(self) -> int:

View File

@ -302,6 +302,7 @@
</div>
</div>
{% with ratings_listed_count=extension.ratings.listed.count %}
<div class="row">
<div class="col-md-8">
{% if my_rating and not my_rating.is_listed %}
@ -311,19 +312,20 @@
{% include "ratings/components/rating.html" with classes="mb-2" %}
{% endfor %}
{% if not extension.ratings.listed.count and not my_rating %}
{% if not ratings_listed_count and not my_rating %}
<a href="{{ extension.get_rate_url }}">{% trans "Be the first to review." %}</a>
{% endif %}
</div>
<div class="col-md-4">
{# Rating #}
{% if extension.ratings.listed.count %}
{% if ratings_listed_count %}
<section>
{% include "ratings/components/summary.html" %}
</section>
{% endif %}
</div>
</div>
{% endwith %}
</section>
{% endblock extension_reviews %}

View File

@ -42,7 +42,10 @@ class ListedExtensionMixin:
"""Fetch a publicly listed extension by slug in the URL before dispatching the view."""
def dispatch(self, *args, **kwargs):
self.extension = get_object_or_404(Extension.objects.listed, slug=self.kwargs['slug'])
self.extension = get_object_or_404(
Extension.objects.listed.prefetch_related('authors', 'ratings'),
slug=self.kwargs['slug'],
)
return super().dispatch(*args, **kwargs)

View File

@ -231,6 +231,8 @@ class ExtensionDetailView(DetailView):
'preview_set',
'preview_set__file',
'ratings',
'ratings__ratingreply',
'ratings__ratingreply__user',
'ratings__user',
'team',
'versions',
@ -255,6 +257,7 @@ class ExtensionDetailView(DetailView):
context['my_rating'] = ratings.models.Rating.get_for(
self.request.user.pk, self.object.pk
)
context['is_maintainer'] = self.object.has_maintainer(self.request.user)
extension = context['object']
# Add the image for "og:image" meta to the context
if extension.featured_image and extension.featured_image.is_listed:

View File

@ -7,40 +7,6 @@ from extensions.models import Version
from .models import Rating
class RatingTypeFilter(admin.SimpleListFilter):
# Human-readable title which will be displayed in the
# right admin sidebar just above the filter options.
title = 'Type'
# Parameter for the filter that will be used in the URL query.
parameter_name = 'type'
def lookups(self, request, model_admin):
"""Return a list of lookup option tuples.
The first element in each tuple is the coded value
for the option that will appear in the URL query.
The second element is the human-readable name for
the option that will appear in the right sidebar.
"""
return (
('rating', 'User Rating'),
('reply', 'Developer/Admin Reply'),
)
def queryset(self, request, queryset):
"""Return the filtered queryset.
Filter based on the value provided in the query string
and retrievable via `self.value()`.
"""
if self.value() == 'rating':
return queryset.filter(reply_to__isnull=True)
elif self.value() == 'reply':
return queryset.filter(reply_to__isnull=False)
return queryset
class RatingAdmin(admin.ModelAdmin):
date_hierarchy = 'date_created'
search_fields = ('extension__slug', 'extension__name', 'user__email', 'user__full_name')
@ -48,7 +14,6 @@ class RatingAdmin(admin.ModelAdmin):
'extension',
'version',
'user',
'reply_to',
)
readonly_fields = (
'date_created',
@ -66,19 +31,17 @@ class RatingAdmin(admin.ModelAdmin):
'user',
'ip_address',
'score',
'is_reply',
'status',
'truncated_text',
)
list_filter = ('status', RatingTypeFilter, 'score')
list_filter = ('status', 'score')
actions = ('delete_selected',)
list_select_related = ('user',) # For extension/reply_to see get_queryset()
list_select_related = ('user',)
def get_queryset(self, request):
base_qs = Rating.objects.all()
return base_qs.prefetch_related(
Prefetch('version', queryset=Version.objects.all()),
Prefetch('reply_to', queryset=base_qs),
)
def has_add_permission(self, request):
@ -87,11 +50,5 @@ class RatingAdmin(admin.ModelAdmin):
def truncated_text(self, obj):
return truncatechars(obj.text, 140) if obj.text else ''
def is_reply(self, obj):
return bool(obj.reply_to)
is_reply.boolean = True
is_reply.admin_order_field = 'reply_to'
admin.site.register(Rating, RatingAdmin)

View File

@ -0,0 +1,58 @@
# Generated by Django 4.2.11 on 2024-06-10 14:01
from django.conf import settings
from django.db import migrations, models
import django.db.models.deletion
class Migration(migrations.Migration):
dependencies = [
migrations.swappable_dependency(settings.AUTH_USER_MODEL),
('ratings', '0006_alter_ratingflag_user'),
]
operations = [
migrations.CreateModel(
name='RatingReply',
fields=[
('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
('date_created', models.DateTimeField(auto_now_add=True)),
('date_modified', models.DateTimeField(auto_now=True)),
('text', models.TextField(null=True)),
],
options={
'abstract': False,
},
),
migrations.RemoveConstraint(
model_name='rating',
name='rating_one_review_per_user_key',
),
migrations.RemoveIndex(
model_name='rating',
name='rating_latest_idx',
),
migrations.RemoveField(
model_name='rating',
name='reply_to',
),
migrations.AddIndex(
model_name='rating',
index=models.Index(fields=['is_latest', 'date_created'], name='rating_latest_idx'),
),
migrations.AddConstraint(
model_name='rating',
constraint=models.UniqueConstraint(fields=('version', 'user'), name='rating_one_review_per_user_key'),
),
migrations.AddField(
model_name='ratingreply',
name='rating',
field=models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, to='ratings.rating'),
),
migrations.AddField(
model_name='ratingreply',
name='user',
field=models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, to=settings.AUTH_USER_MODEL),
),
]

View File

@ -14,14 +14,13 @@ log = logging.getLogger(__name__)
class RatingManager(models.Manager):
# TODO: figure out how to retrieve reviews "annotated" with replies, if any
@property
def listed(self):
return self.filter(status=self.model.STATUSES.APPROVED, reply_to__isnull=True)
return self.filter(status=self.model.STATUSES.APPROVED)
@property
def unlisted(self):
return self.exclude(status=self.models.STATUSES.APPROVED, reply_to__isnull=True)
return self.exclude(status=self.models.STATUSES.APPROVED)
@property
def listed_texts(self):
@ -41,12 +40,6 @@ class Rating(CreatedModifiedMixin, TrackChangesMixin, models.Model):
'extensions.Version', related_name='ratings', on_delete=models.CASCADE
)
user = models.ForeignKey(User, related_name='ratings', on_delete=models.CASCADE)
reply_to = models.OneToOneField(
'self',
null=True,
related_name='reply',
on_delete=models.CASCADE,
)
score = models.PositiveSmallIntegerField(null=True, choices=SCORES)
text = models.TextField(null=True)
@ -74,14 +67,14 @@ class Rating(CreatedModifiedMixin, TrackChangesMixin, models.Model):
models.Index(fields=('version',), name='rating_version_id'),
models.Index(fields=('user',), name='rating_user_idx'),
models.Index(
fields=('reply_to', 'is_latest', 'date_created'),
fields=('is_latest', 'date_created'),
name='rating_latest_idx',
),
models.Index(fields=('ip_address',), name='rating_ip_address_idx'),
]
constraints = [
models.UniqueConstraint(
fields=('version', 'user', 'reply_to'), name='rating_one_review_per_user_key'
fields=('version', 'user'), name='rating_one_review_per_user_key'
),
]
@ -98,7 +91,6 @@ class Rating(CreatedModifiedMixin, TrackChangesMixin, models.Model):
def get_for(cls, user_id: int, extension_id: int):
"""Get rating left by a given user for a given extension."""
return cls.objects.filter(
reply_to=None,
user_id=user_id,
extension_id=extension_id,
).first()
@ -131,12 +123,6 @@ class Rating(CreatedModifiedMixin, TrackChangesMixin, models.Model):
# user_responsible=user.
self.save()
@classmethod
def get_replies(cls, ratings):
ratings = [r.id for r in ratings]
qs = Rating.objects.filter(reply_to__in=ratings)
return {r.reply_to_id: r for r in qs}
def post_save(sender, instance, created, **kwargs):
if kwargs.get('raw'):
return
@ -147,9 +133,6 @@ class Rating(CreatedModifiedMixin, TrackChangesMixin, models.Model):
# when manipulating a Rating that indicates a real user made that
# change.
action = 'New' if created else 'Edited'
if instance.reply_to:
log.info(f'{action} reply to {instance.reply_to_id}: {instance.pk}')
else:
log.info(f'{action} rating: {instance.pk}')
def get_report_url(self):
@ -163,6 +146,16 @@ class Rating(CreatedModifiedMixin, TrackChangesMixin, models.Model):
],
)
def get_reply_url(self):
return reverse(
'ratings:reply',
args=[
self.extension.type_slug,
self.extension.slug,
self.id,
],
)
class RatingFlag(CreatedModifiedMixin, models.Model):
SPAM = 'review_flag_reason_spam'
@ -190,3 +183,9 @@ class RatingFlag(CreatedModifiedMixin, models.Model):
constraints = [
models.UniqueConstraint(fields=('rating', 'user'), name='ratingflag_review_user_key')
]
class RatingReply(CreatedModifiedMixin, models.Model):
rating = models.OneToOneField(Rating, on_delete=models.CASCADE)
user = models.ForeignKey(User, null=True, on_delete=models.SET_NULL)
text = models.TextField(null=True)

View File

@ -43,8 +43,30 @@
</a>
{% endif %}
{% if is_maintainer and not rating.ratingreply %}
<a href="{{ rating.get_reply_url }}" title="Reply">
<i class="i-reply"></i>
</a>
{% endif %}
</ul>
</header>
<div>{{ rating.text|markdown }}</div>
{% if rating.ratingreply %}
<div class="rating-reply">
<header>
<ul>
<li class="align-items-center me-auto">
<a href="{% url "extensions:by-author" user_id=rating.ratingreply.user.pk %}">{{ rating.ratingreply.user }}</a>
replied
</li>
<li>
{{ rating.ratingreply.date_created|naturaltime_compact }}
</li>
</ul>
</header>
{{ rating.ratingreply.text|markdown }}
</div>
{% endif %}
</div>
</article>

View File

@ -2,7 +2,8 @@
{% has_maintainer extension as is_maintainer %}
<div class="card p-3 mb-3 mt-2 mt-lg-0 ratings-summary">
{% if extension.text_ratings_count %}
{% with text_ratings_count=extension.text_ratings_count %}
{% if text_ratings_count %}
<div class="summary-container">
<div class="summary-value">
<h3>
@ -13,7 +14,7 @@
{% include "ratings/components/average.html" with score=extension.average_score %}
<a class="text-muted d-block" href="{{ extension.get_ratings_url }}">
<small>{{ extension.text_ratings_count }} review{{ extension.text_ratings_count | pluralize }}</small>
<small>{{ text_ratings_count }} review{{ text_ratings_count | pluralize }}</small>
</a>
</div>
<div class="summary-bars">
@ -36,6 +37,7 @@
<span class="text-muted">Be the first to review.</span>
{% else %}
{% endif %}
{% endwith %}
<div class="mt-3">
{% if not is_maintainer and not my_rating %}

View File

@ -1,6 +1,5 @@
{% extends "extensions/base.html" %}
{% load i18n %}
{% load humanize %}
{% load extensions humanize i18n %}
{% block page_title %}{{ extension.name }}{% endblock page_title %}
@ -8,9 +7,11 @@
{% with latest=extension.latest_version author=extension.latest_version.file.user %}
<div class="d-flex mt-4">
<h2>
{% if extension.text_ratings_count and not score %}
{{ extension.text_ratings_count }} {% endif %}Reviews{% if score %} with {{ score|apnumber }} stars
{% with text_ratings_count=extension.text_ratings_count %}
{% if text_ratings_count and not score %}
{{ text_ratings_count }} {% endif %}Reviews{% if score %} with {{ score|apnumber }} stars
{% endif %}
{% endwith %}
</h2>
{% if score %}
<div class="ms-auto">

View File

@ -0,0 +1,36 @@
{% extends "extensions/base.html" %}
{% load i18n common %}
{% block page_title %}Reply to rating{% endblock page_title %}
{% block content %}
<h2>Reply to review</h2>
<div class="row">
<div class="col-md-8">
{% include "ratings/components/rating.html" with classes="mb-2" %}
</div>
</div>
<div class="rating-form">
<form method="post" enctype="multipart/form-data">
{% csrf_token %}
{% with form=form|add_form_classes %}
<div class="row">
<div class="col-md-8">
<div class="box p-3">
{% include "common/components/field.html" with field=form.text focus=True placeholder="Enter your reply here..." %}
{% if form.non_field_errors %}
<div class="invalid-feedback">
{{ form.non_field_errors }}
</div>
{% endif %}
<button type="submit" class="btn btn-block btn-primary mt-4">
<i class="i-send"></i>
<span>{% trans 'Submit Reply' %}</span>
</button>
</div>
</div>
</div>
{% endwith %}
</form>
</div>
{% endblock content %}

View File

@ -163,3 +163,18 @@ class AddRatingViewTest(TestCase):
rating = Rating.objects.first()
self.assertEqual(rating.score, 3)
self.assertEqual(rating.text, text)
def test_reply(self):
version = create_approved_version(ratings=[])
rating = RatingFactory(version=version, text='some text', status=Rating.STATUSES.APPROVED)
random_user = UserFactory()
self.client.force_login(random_user)
response = self.client.post(rating.get_reply_url(), {'text': 'some reply'})
self.assertEqual(response.status_code, 403)
self.client.force_login(version.extension.authors.first())
response = self.client.post(rating.get_reply_url(), {'text': 'some reply'})
self.assertEqual(response.status_code, 302)
rating.refresh_from_db()
self.assertTrue(hasattr(rating, 'ratingreply'))

View File

@ -11,6 +11,11 @@ urlpatterns = [
[
path('<slug:slug>/reviews/', views.RatingsView.as_view(), name='for-extension'),
path('<slug:slug>/reviews/new/', views.AddRatingView.as_view(), name='new'),
path(
'<slug:slug>/reviews/<int:pk>/reply/',
views.AddRatingReplyView.as_view(),
name='reply',
),
]
),
)

View File

@ -5,7 +5,7 @@ from django.views.generic.edit import CreateView
from django.views.generic.list import ListView
from ratings.forms import AddRatingForm
from ratings.models import Rating
from ratings.models import Rating, RatingReply
import extensions.views.mixins
log = logging.getLogger(__name__)
@ -31,7 +31,18 @@ class RatingsView(extensions.views.mixins.ListedExtensionMixin, ListView):
def get_queryset(self):
self._set_score_filter()
queryset = super().get_queryset().filter(extension_id=self.extension.pk)
queryset = (
super()
.get_queryset()
.filter(extension_id=self.extension.pk)
.select_related(
'extension',
'ratingreply',
'ratingreply__user',
'user',
'version',
)
)
if self.score:
queryset = queryset.filter(score=self.score)
return queryset.distinct()
@ -39,6 +50,7 @@ class RatingsView(extensions.views.mixins.ListedExtensionMixin, ListView):
def get_context_data(self, **kwargs):
context = super().get_context_data(**kwargs)
context['extension'] = self.extension
context['is_maintainer'] = self.extension.has_maintainer(self.request.user)
context['score'] = self.score
if self.request.user.is_authenticated:
context['my_rating'] = Rating.get_for(self.request.user.pk, self.extension.pk)
@ -77,3 +89,39 @@ class AddRatingView(
def get_success_url(self) -> str:
return self.extension.get_ratings_url()
class AddRatingReplyView(
extensions.views.mixins.ListedExtensionMixin,
UserPassesTestMixin,
LoginRequiredMixin,
CreateView,
):
model = RatingReply
fields = ('text',)
def get_rating(self):
return Rating.objects.filter(pk=self.kwargs['pk'], extension=self.extension).first()
def test_func(self) -> bool:
# Check that user is replying to a rating for their exension and this is the first reply
rating = self.get_rating()
return (
rating
and not hasattr(rating, 'ratingreply')
and self.extension.has_maintainer(self.request.user)
)
def form_valid(self, form):
form.instance.rating = self.get_rating()
form.instance.user = self.request.user
return super().form_valid(form)
def get_context_data(self, **kwargs):
context = super().get_context_data(**kwargs)
context['extension'] = self.extension
context['rating'] = self.get_rating()
return context
def get_success_url(self) -> str:
return self.extension.get_ratings_url()

View File

@ -106,6 +106,7 @@ class ExtensionsApprovalDetailView(DetailView):
filtered_activity_types = {ApprovalActivity.ActivityType.COMMENT}
user = self.request.user
if self.object.has_maintainer(user):
ctx['is_maintainer'] = self.object.has_maintainer(self.request.user)
filtered_activity_types.add(ApprovalActivity.ActivityType.AWAITING_REVIEW)
if user.is_moderator or user.is_superuser:
filtered_activity_types.update(