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% width: 100%
.rating-form .rating-form
.box
margin-left: calc(var(--spacer) * 2.5)
select select
color: var(--color-warning) color: var(--color-warning)
@ -391,6 +394,15 @@
&:focus &:focus
color: var(--color-warning) 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 // TODO: consider adding component boxed nav generic to web-assets, and make variants on top of that
.nav-pills .nav-pills
@extend .dropdown-menu @extend .dropdown-menu

View File

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

View File

@ -302,6 +302,7 @@
</div> </div>
</div> </div>
{% with ratings_listed_count=extension.ratings.listed.count %}
<div class="row"> <div class="row">
<div class="col-md-8"> <div class="col-md-8">
{% if my_rating and not my_rating.is_listed %} {% if my_rating and not my_rating.is_listed %}
@ -311,19 +312,20 @@
{% include "ratings/components/rating.html" with classes="mb-2" %} {% include "ratings/components/rating.html" with classes="mb-2" %}
{% endfor %} {% 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> <a href="{{ extension.get_rate_url }}">{% trans "Be the first to review." %}</a>
{% endif %} {% endif %}
</div> </div>
<div class="col-md-4"> <div class="col-md-4">
{# Rating #} {# Rating #}
{% if extension.ratings.listed.count %} {% if ratings_listed_count %}
<section> <section>
{% include "ratings/components/summary.html" %} {% include "ratings/components/summary.html" %}
</section> </section>
{% endif %} {% endif %}
</div> </div>
</div> </div>
{% endwith %}
</section> </section>
{% endblock extension_reviews %} {% 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.""" """Fetch a publicly listed extension by slug in the URL before dispatching the view."""
def dispatch(self, *args, **kwargs): 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) return super().dispatch(*args, **kwargs)

View File

@ -231,6 +231,8 @@ class ExtensionDetailView(DetailView):
'preview_set', 'preview_set',
'preview_set__file', 'preview_set__file',
'ratings', 'ratings',
'ratings__ratingreply',
'ratings__ratingreply__user',
'ratings__user', 'ratings__user',
'team', 'team',
'versions', 'versions',
@ -255,6 +257,7 @@ class ExtensionDetailView(DetailView):
context['my_rating'] = ratings.models.Rating.get_for( context['my_rating'] = ratings.models.Rating.get_for(
self.request.user.pk, self.object.pk self.request.user.pk, self.object.pk
) )
context['is_maintainer'] = self.object.has_maintainer(self.request.user)
extension = context['object'] extension = context['object']
# Add the image for "og:image" meta to the context # Add the image for "og:image" meta to the context
if extension.featured_image and extension.featured_image.is_listed: 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 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): class RatingAdmin(admin.ModelAdmin):
date_hierarchy = 'date_created' date_hierarchy = 'date_created'
search_fields = ('extension__slug', 'extension__name', 'user__email', 'user__full_name') search_fields = ('extension__slug', 'extension__name', 'user__email', 'user__full_name')
@ -48,7 +14,6 @@ class RatingAdmin(admin.ModelAdmin):
'extension', 'extension',
'version', 'version',
'user', 'user',
'reply_to',
) )
readonly_fields = ( readonly_fields = (
'date_created', 'date_created',
@ -66,19 +31,17 @@ class RatingAdmin(admin.ModelAdmin):
'user', 'user',
'ip_address', 'ip_address',
'score', 'score',
'is_reply',
'status', 'status',
'truncated_text', 'truncated_text',
) )
list_filter = ('status', RatingTypeFilter, 'score') list_filter = ('status', 'score')
actions = ('delete_selected',) actions = ('delete_selected',)
list_select_related = ('user',) # For extension/reply_to see get_queryset() list_select_related = ('user',)
def get_queryset(self, request): def get_queryset(self, request):
base_qs = Rating.objects.all() base_qs = Rating.objects.all()
return base_qs.prefetch_related( return base_qs.prefetch_related(
Prefetch('version', queryset=Version.objects.all()), Prefetch('version', queryset=Version.objects.all()),
Prefetch('reply_to', queryset=base_qs),
) )
def has_add_permission(self, request): def has_add_permission(self, request):
@ -87,11 +50,5 @@ class RatingAdmin(admin.ModelAdmin):
def truncated_text(self, obj): def truncated_text(self, obj):
return truncatechars(obj.text, 140) if obj.text else '' 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) 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): class RatingManager(models.Manager):
# TODO: figure out how to retrieve reviews "annotated" with replies, if any
@property @property
def listed(self): def listed(self):
return self.filter(status=self.model.STATUSES.APPROVED, reply_to__isnull=True) return self.filter(status=self.model.STATUSES.APPROVED)
@property @property
def unlisted(self): def unlisted(self):
return self.exclude(status=self.models.STATUSES.APPROVED, reply_to__isnull=True) return self.exclude(status=self.models.STATUSES.APPROVED)
@property @property
def listed_texts(self): def listed_texts(self):
@ -41,12 +40,6 @@ class Rating(CreatedModifiedMixin, TrackChangesMixin, models.Model):
'extensions.Version', related_name='ratings', on_delete=models.CASCADE 'extensions.Version', related_name='ratings', on_delete=models.CASCADE
) )
user = models.ForeignKey(User, 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) score = models.PositiveSmallIntegerField(null=True, choices=SCORES)
text = models.TextField(null=True) 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=('version',), name='rating_version_id'),
models.Index(fields=('user',), name='rating_user_idx'), models.Index(fields=('user',), name='rating_user_idx'),
models.Index( models.Index(
fields=('reply_to', 'is_latest', 'date_created'), fields=('is_latest', 'date_created'),
name='rating_latest_idx', name='rating_latest_idx',
), ),
models.Index(fields=('ip_address',), name='rating_ip_address_idx'), models.Index(fields=('ip_address',), name='rating_ip_address_idx'),
] ]
constraints = [ constraints = [
models.UniqueConstraint( 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): def get_for(cls, user_id: int, extension_id: int):
"""Get rating left by a given user for a given extension.""" """Get rating left by a given user for a given extension."""
return cls.objects.filter( return cls.objects.filter(
reply_to=None,
user_id=user_id, user_id=user_id,
extension_id=extension_id, extension_id=extension_id,
).first() ).first()
@ -131,12 +123,6 @@ class Rating(CreatedModifiedMixin, TrackChangesMixin, models.Model):
# user_responsible=user. # user_responsible=user.
self.save() 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): def post_save(sender, instance, created, **kwargs):
if kwargs.get('raw'): if kwargs.get('raw'):
return return
@ -147,9 +133,6 @@ class Rating(CreatedModifiedMixin, TrackChangesMixin, models.Model):
# when manipulating a Rating that indicates a real user made that # when manipulating a Rating that indicates a real user made that
# change. # change.
action = 'New' if created else 'Edited' 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}') log.info(f'{action} rating: {instance.pk}')
def get_report_url(self): 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): class RatingFlag(CreatedModifiedMixin, models.Model):
SPAM = 'review_flag_reason_spam' SPAM = 'review_flag_reason_spam'
@ -190,3 +183,9 @@ class RatingFlag(CreatedModifiedMixin, models.Model):
constraints = [ constraints = [
models.UniqueConstraint(fields=('rating', 'user'), name='ratingflag_review_user_key') 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> </a>
{% endif %} {% endif %}
{% if is_maintainer and not rating.ratingreply %}
<a href="{{ rating.get_reply_url }}" title="Reply">
<i class="i-reply"></i>
</a>
{% endif %}
</ul> </ul>
</header> </header>
<div>{{ rating.text|markdown }}</div> <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> </div>
</article> </article>

View File

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

View File

@ -1,6 +1,5 @@
{% extends "extensions/base.html" %} {% extends "extensions/base.html" %}
{% load i18n %} {% load extensions humanize i18n %}
{% load humanize %}
{% block page_title %}{{ extension.name }}{% endblock page_title %} {% block page_title %}{{ extension.name }}{% endblock page_title %}
@ -8,9 +7,11 @@
{% with latest=extension.latest_version author=extension.latest_version.file.user %} {% with latest=extension.latest_version author=extension.latest_version.file.user %}
<div class="d-flex mt-4"> <div class="d-flex mt-4">
<h2> <h2>
{% if extension.text_ratings_count and not score %} {% with text_ratings_count=extension.text_ratings_count %}
{{ extension.text_ratings_count }} {% endif %}Reviews{% if score %} with {{ score|apnumber }} stars {% if text_ratings_count and not score %}
{{ text_ratings_count }} {% endif %}Reviews{% if score %} with {{ score|apnumber }} stars
{% endif %} {% endif %}
{% endwith %}
</h2> </h2>
{% if score %} {% if score %}
<div class="ms-auto"> <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() rating = Rating.objects.first()
self.assertEqual(rating.score, 3) self.assertEqual(rating.score, 3)
self.assertEqual(rating.text, text) 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/', views.RatingsView.as_view(), name='for-extension'),
path('<slug:slug>/reviews/new/', views.AddRatingView.as_view(), name='new'), 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 django.views.generic.list import ListView
from ratings.forms import AddRatingForm from ratings.forms import AddRatingForm
from ratings.models import Rating from ratings.models import Rating, RatingReply
import extensions.views.mixins import extensions.views.mixins
log = logging.getLogger(__name__) log = logging.getLogger(__name__)
@ -31,7 +31,18 @@ class RatingsView(extensions.views.mixins.ListedExtensionMixin, ListView):
def get_queryset(self): def get_queryset(self):
self._set_score_filter() 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: if self.score:
queryset = queryset.filter(score=self.score) queryset = queryset.filter(score=self.score)
return queryset.distinct() return queryset.distinct()
@ -39,6 +50,7 @@ class RatingsView(extensions.views.mixins.ListedExtensionMixin, ListView):
def get_context_data(self, **kwargs): def get_context_data(self, **kwargs):
context = super().get_context_data(**kwargs) context = super().get_context_data(**kwargs)
context['extension'] = self.extension context['extension'] = self.extension
context['is_maintainer'] = self.extension.has_maintainer(self.request.user)
context['score'] = self.score context['score'] = self.score
if self.request.user.is_authenticated: if self.request.user.is_authenticated:
context['my_rating'] = Rating.get_for(self.request.user.pk, self.extension.pk) 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: def get_success_url(self) -> str:
return self.extension.get_ratings_url() 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} filtered_activity_types = {ApprovalActivity.ActivityType.COMMENT}
user = self.request.user user = self.request.user
if self.object.has_maintainer(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) filtered_activity_types.add(ApprovalActivity.ActivityType.AWAITING_REVIEW)
if user.is_moderator or user.is_superuser: if user.is_moderator or user.is_superuser:
filtered_activity_types.update( filtered_activity_types.update(