allow posting reviews and replies (bug 557879)

apps/addons/templates/addons/details.html

@@ -172,7 +172,7 @@
     {# /beta #}
   </div>{# /article #}
 
-  {{ review_list_box(addon=addon, reviews=addon.reviews) }}
+  {{ review_list_box(addon=addon, reviews=reviews) }}
 
   {{ review_add_box(addon=addon) }}
 

apps/addons/templates/addons/review_add_box.html

@@ -11,31 +11,11 @@
       </p>
     {% endif %}
 
-    {# TODO uncakeify  #}
-    <form class="addon-feedback" method="post"
-      action="{{ remora_url('/reviews/add/') }}/{{ addon.id }}">
-      {{ cake_csrf_token() }}
-      {% with disabled = ('' if user.is_authenticated() else ' disabled') %}
-        <div class="container">
-          <label for="review">{{ _('Review') }}</label>
-          <textarea name="data[Review][body]" id="review"{{ disabled }}
-                    cols="30" rows="6"></textarea>
-        </div>
-        <div class="container">
-          <label for="review-rating">{{ _('Rating', 'advanced_search_form_rating') }}</label>
-          <select id="review-rating" name="data[Review][rating]">
-            <option></option>
-            <option value="1" class="worst">*</option>
-            <option value="2" class="bad">**</option>
-            <option value="3" class="fair">***</option>
-            <option value="4" class="good">****</option>
-            <option value="5" class="best">*****</option>
-          </select>
-          <input type="hidden" name="data[Review][id]" value="" id="ReviewId"/>
-          <input type="hidden" name="data[Review][title]" value=" " id="ReviewTitle"/>
-          <button type="submit"{{ disabled }}>{{ _('Post Review') }}</button>
-        </div>
-      {% endwith %}
+    <form method="post" action="{{ url('reviews.add', addon.id) }}">
+      {{ csrf() }}
+      {{ field(review_form.body, _('Review:')) }}
+      {{ field(review_form.rating, _('Rating:')) }}
+      <input type="submit" value="{{ _('Submit review') }}">
     </form>
 
     <p>
@@ -57,7 +37,7 @@
     <p><a href="{{ remora_url('/pages/review_guide') }}">{{ _('Review Guidelines') }}</a></p>
     <p>
       {# TODO reverse url #}
-      <a href="{{ remora_url('/reviews/add/{0}'|f(addon.id)) }}">
+      <a href="{{ url('reviews.add', addon.id) }}">
         {{ _('Detailed Review') }}</a>
     </p>
 

apps/addons/tests/test_views.py

@@ -506,12 +506,11 @@ class TestDetailPage(amo.test_utils.ExtraSetup, test_utils.TestCase):
         assert len(doc('#tags li input.removetag'))
 
     def test_detailed_review_link(self):
-        # TODO(jbalogh): use reverse when we drop remora.
         self.client.login(username='regular@mozilla.com', password='password')
         r = self.client.get(reverse('addons.detail', args=[3615]))
         doc = pq(r.content)
         href = doc('#review-box a[href*="reviews/add"]').attr('href')
-        assert href.endswith('/reviews/add/3615'), href
+        assert href.endswith(reverse('reviews.add', args=[3615])), href
 
     def test_no_listed_authors(self):
         r = self.client.get(reverse('addons.detail', args=[59]))

apps/addons/views.py

@@ -19,6 +19,8 @@ from amo.helpers import absolutify
 from amo import urlresolvers
 from amo.urlresolvers import reverse
 from bandwagon.models import Collection, CollectionFeature, CollectionPromo
+from reviews.forms import ReviewForm
+from reviews.models import Review
 from stats.models import GlobalStat, Contribution
 from tags.models import Tag
 from translations.query import order_by_translation
@@ -127,6 +129,8 @@ def extension_detail(request, addon):
         'current_user_tags': current_user_tags,
 
         'recommendations': recommended,
+        'review_form': ReviewForm(),
+        'reviews': Review.objects.valid().filter(addon=addon, is_latest=True),
 
         'collections': popular_coll,
         'other_collection_count': other_coll_count,

apps/amo/helpers.py

@@ -2,7 +2,6 @@ import collections
 import json as jsonlib
 import math
 import random
-import urllib
 
 from django.conf import settings
 from django.utils import translation
@@ -293,3 +292,10 @@ def license_link(license):
     parts.append('</li></ul>')
 
     return jinja2.Markup(''.join(parts))
+
+
+@register.function
+def field(field, label=None):
+    if label is not None:
+        field.label = label
+    return jinja2.Markup(u'%s%s%s' % (field.errors, field.label_tag(), field))

apps/reviews/forms.py

@@ -3,6 +3,15 @@ from django import forms
 from .models import ReviewFlag
 
 
+class ReviewReplyForm(forms.Form):
+    title = forms.CharField(required=False)
+    body = forms.CharField(widget=forms.Textarea(attrs={'rows': 3}))
+
+
+class ReviewForm(ReviewReplyForm):
+    rating = forms.ChoiceField(zip(range(1, 6), range(1, 6)))
+
+
 class ReviewFlagForm(forms.ModelForm):
 
     class Meta:

apps/reviews/helpers.py

@@ -25,3 +25,9 @@ def reviews_link(addon, collection_uuid=None):
     t = jingo.env.get_template('reviews/reviews_link.html')
     return jinja2.Markup(t.render(addon=addon,
                                   collection_uuid=collection_uuid))
+
+
+@jingo.register.inclusion_tag('reviews/report_review.html')
+@jinja2.contextfunction
+def report_review_popup(context):
+    return context

apps/reviews/models.py

@@ -31,7 +31,7 @@ class Review(amo.models.ModelBase):
                                 null=True)
     user = models.ForeignKey('users.UserProfile', related_name='_reviews_all')
     reply_to = models.ForeignKey('self', null=True, unique=True,
-                                 db_column='reply_to')
+                                 related_name='replies', db_column='reply_to')
 
     rating = models.PositiveSmallIntegerField(null=True)
     title = TranslatedField()

apps/reviews/tasks.py

@@ -20,7 +20,8 @@ def update_denorm(*pairs, **kw):
     log.info('[%s@%s] Updating review denorms.' %
              (len(pairs), update_denorm.rate_limit))
     for addon, user in pairs:
-        reviews = list(Review.uncached.filter(addon=addon, user=user)
+        reviews = list(Review.objects.valid().no_cache()
+                       .filter(addon=addon, user=user)
                        .filter(reply_to=None).order_by('created'))
         if not reviews:
             continue

apps/reviews/templates/reviews/add.html

@@ -0,0 +1,52 @@
+{% extends "base_side_categories.html" %}
+
+{% set title = _('Add a review for {0}')|f(addon.name) %}
+
+{% block title %}{{ page_title(title) }}{% endblock %}
+
+{% block content %}
+
+<header>
+  {{ breadcrumbs([(addon.type_url(), amo.ADDON_TYPES[addon.type]),
+                  (addon.get_url_path(), addon.name),
+                  (url('reviews.list', addon.id), _('Reviews')),
+                  (None, _('Add'))]) }}
+  <h2>{{ title }}</h2>
+</header>
+
+<div class="prose">
+  {% trans support=addon.get_url_path() + "#support",
+           guide=remora_url('pages/review_guide') %}
+  <p>Keep these tips in mind:</p>
+  <ul>
+    <li>
+      Write like you're telling a friend about your experience with the add-on.
+      Give specifics and helpful details, such as what features you liked and/or
+      disliked, how easy to use it is, and any disadvantages it has.  Avoid generic
+      language such as calling it "Great" or "Bad" unless you can give reasons why
+      you believe this is so.
+    </li>
+    <li>
+      Please do not post bug reports in reviews. We do not make your email
+      address available to add-on developers and they may need to contact you to help
+      resolve your issue. See the <a href="{{ support }}">support section</a> to find out
+      where to get assistance for this add-on.
+    </li>
+    <li>Please keep reviews clean, avoid the use of improper language and do not
+        post any personal information.
+    </li>
+  </ul>
+  <p>Please read the <a href="{{ guide }}">Review Guidelines</a> for more detail
+  about user add-on reviews.</p>
+  {% endtrans %}
+</div>
+
+<form method="post" action="{{ url('reviews.add', addon.id) }}">
+  {{ csrf() }}
+  {{ field(form.title, _('Title:')) }}
+  {{ field(form.rating, _('Rating:')) }}
+  {{ field(form.body, _('Review:')) }}
+  <input type="submit" value="{{ _('Submit review') }}">
+</form>
+
+{% endblock %}

apps/reviews/templates/reviews/reply.html

@@ -0,0 +1,29 @@
+{% extends "reviews/review_list.html" %}
+
+{% set title = _('Reply to review by {0}')|f(review.user.display_name) %}
+
+{% block title %}{{ page_title(title) }}{% endblock %}
+
+{% block content %}
+  {% block review_header %}
+    <h2>{{ title }}</h2>
+  {% endblock %}
+
+  {% block review_list %}
+    {% include "reviews/review.html" %}
+    <div class="review article reply-form">
+      <h3>{{ _('Write a Reply') }}</h3>
+      <form method="post" action="{{ url('reviews.reply', addon.id, review.id) }}">
+        {{ csrf() }}
+        {{ field(form.title, _('Title:')) }}
+        {{ field(form.body, _('Reply:')) }}
+        <input type="submit" value="{{ _('Submit Your Reply') }}">
+        {# L10n: this string is following a <button>. #}
+        {% trans url=url('reviews.list', addon.id) %}
+          or <a href="{{ url }}">Cancel</a>
+        {% endtrans %}
+      </form>
+    </div>
+  {% endblock %}
+{{ report_review_popup() }}
+{% endblock %}

apps/reviews/templates/reviews/report_review.html

@@ -0,0 +1,15 @@
+<div class="hidden">
+  <div class="install-note review-reason">
+    <strong>{{ _('Please select a reason:') }}</strong>
+    <ul>
+      {% for flag, text in ReviewFlag.FLAGS %}
+        <li><a href="#{{ flag }}">{{ text }}</a></li>
+      {% endfor %}
+    </ul>
+    {# Using a fake form so we get submission on keyboard-enter. #}
+    <form class="other-note" method="POST" action="">
+      {{ flag_form.note|safe }}
+      <input type="submit" value="{{ _('Submit') }}">
+    </form>
+  </div>
+</div>

apps/reviews/templates/reviews/review.html

@@ -1,3 +1,4 @@
+{% set perms = review_perms or {} %}
 {% set outdated = (review.version_id
                       and review.version_id != addon._current_version_id) %}
 {% set is_reply = review.reply_to_id is not none %}
@@ -50,8 +51,11 @@
           {{ _('Report this review') }}</a>
       </li>
     {% endif %}
-    {% if perms.is_author or perms.is_admin %}
-      <li><a href="#TODO">{{ _('Reply to review') }}</a></li>
+    {% if not is_reply and (perms.is_author or perms.is_admin) %}
+      <li>
+        <a class="review-delete" href="{{ url('reviews.reply', addon.id, review.id) }}">
+          {{ _('Reply to review') }}</a>
+      </li>
     {% endif %}
     {% if perms.can_delete %}
       <li>

apps/reviews/templates/reviews/review_list.html

@@ -12,6 +12,7 @@
 
 {% block content %}
 <div class="primary" role="main">
+  {% block review_header %}
   <header>
     {# Give a link back to reviews if we're looking at user reviews or a detail page. #}
     {% with link = None if page == 'list' else url('reviews.list', addon.id) %}
@@ -43,18 +44,9 @@
       {% endif %}
     </hgroup>
   </header>
-  <div id="dev-reply-form" class="review article reply reply-form hidden">
-    <h3>Write a Reply</h3>
-    <form method="POST" action="">
-      <label for="reply-title">{{_('Title:')}}</label>
-      <input id="reply-title" type="text" name="title" />
-      <label for="reply-body">{{_('Reply:')}}</label>
-      <textarea id="reply-body" name="body"></textarea>
-      <input type="hidden" name="inreplyto" value="" />
-      <input type="submit" value="{{ _('Submit Your Reply') }}">
-      or <a href="#">Cancel</a>
-    </form>
-  </div>
+  {% endblock %}
+
+  {% block review_list %}
   {% for review in reviews.object_list %}
     {% include "reviews/review.html" %}
     {% if review.id in replies %}
@@ -64,6 +56,7 @@
     {% endif %}
   {% endfor %}
   {{ reviews|paginator }}
+  {% endblock review_list %}
 </div>
 
 <div class="secondary">
@@ -92,26 +85,14 @@
         </ul>
       </div>
     {% endif %}
-    <div>
-      <a class="button" href="#TODO">
-        {{ _('Write a New Review') }}</a>
-    </div>
+    {% if not review_perms.is_author %}
+      <div>
+        <a class="button" href="{{ url('reviews.add', addon.id) }}">
+          {{ _('Write a New Review') }}</a>
+      </div>
+    {% endif %}
   </div>
 </div>
 
-<div class="hidden">
-  <div class="install-note review-reason">
-    <strong>{{ _('Please select a reason:') }}</strong>
-    <ul>
-      {% for flag, text in ReviewFlag.FLAGS %}
-        <li><a href="#{{ flag }}">{{ text }}</a></li>
-      {% endfor %}
-    </ul>
-    {# Using a fake form so we get submission on keyboard-enter. #}
-    <form class="other-note" method="POST" action="">
-      {{ flag_form.note|safe }}
-      <input type="submit" value="{{ _('Submit') }}">
-    </form>
-  </div>
-</div>
+{{ report_review_popup() }}
 {% endblock content %}

apps/reviews/tests/test_cron.py

@@ -36,7 +36,7 @@ class TestDenormalization(test_utils.TestCase):
 
     def test_denorm_on_save(self):
         addon, user = Review.objects.values_list('addon', 'user')[0]
-        Review.objects.create(addon_id=addon, user_id=user)
+        Review.objects.create(addon_id=addon, user_id=user, rating=3)
         self._check()
 
     def test_denorm_on_delete(self):

apps/reviews/urls.py

@@ -6,12 +6,14 @@ from . import views
 # These all start with /addon/:id/reviews/:review_id/.
 detail_patterns = patterns('',
     url('^$', views.review_list, name='reviews.detail'),
+    url('^reply$', views.reply, name='reviews.reply'),
     url('^flag$', views.flag, name='reviews.flag'),
     url('^delete$', views.delete, name='reviews.delete'),
 )
 
 urlpatterns = patterns('',
     url('^$', views.review_list, name='reviews.list'),
+    url('^add$', views.add, name='reviews.add'),
     url('^(?P<review_id>\d+)/', include(detail_patterns)),
     url('^format:rss$', ReviewsRss(), name='reviews.list.rss'),
     url('^user:(?P<user_id>\d+)$', views.review_list, name='reviews.user'),

apps/reviews/views.py

@@ -1,6 +1,6 @@
 from django import http
 from django.contrib.auth.decorators import login_required
-from django.shortcuts import get_object_or_404
+from django.shortcuts import get_object_or_404, redirect
 
 import commonware.log
 import jingo
@@ -12,19 +12,24 @@ from access import acl
 from addons.models import Addon
 
 from .models import Review, ReviewFlag, GroupedRating
-from .forms import ReviewFlagForm
+from . import forms
 
 log = commonware.log.getLogger('z.reviews')
 
 
+def flag_context():
+    return dict(ReviewFlag=ReviewFlag,
+                flag_form=forms.ReviewFlagForm())
+
+
 def review_list(request, addon_id, review_id=None, user_id=None):
     addon = get_object_or_404(Addon.objects.valid(), id=addon_id)
     q = (Review.objects.valid().filter(addon=addon)
          .order_by('-created'))
 
-    ctx = {'addon': addon, 'ReviewFlag': ReviewFlag,
-           'flag_form': ReviewFlagForm(),
+    ctx = {'addon': addon,
            'grouped_ratings': GroupedRating.get(addon_id)}
+    ctx.update(flag_context())
 
     if review_id is not None:
         ctx['page'] = 'detail'
@@ -44,7 +49,7 @@ def review_list(request, addon_id, review_id=None, user_id=None):
     ctx['reviews'] = reviews = amo.utils.paginate(request, q)
     ctx['replies'] = get_replies(reviews.object_list)
     if request.user.is_authenticated():
-        ctx['perms'] = {
+        ctx['review_perms'] = {
             'is_admin': acl.action_allowed(request, 'Admin', 'EditAnyAddon'),
             'is_editor': acl.action_allowed(request, 'Editor', '%'),
             'is_author': acl.check_ownership(request, addon,
@@ -53,6 +58,8 @@ def review_list(request, addon_id, review_id=None, user_id=None):
                                              'DeleteReview'),
         }
         ctx['flags'] = get_flags(request, reviews.object_list)
+    else:
+        ctx['review_perms'] = {}
     return jingo.render(request, 'reviews/review_list.html', ctx)
 
 
@@ -78,7 +85,7 @@ def flag(request, addon_id, review_id):
     except ReviewFlag.DoesNotExist:
         instance = None
     data = dict(request.POST.items(), **d)
-    form = ReviewFlagForm(data, instance=instance)
+    form = forms.ReviewFlagForm(data, instance=instance)
     if form.is_valid():
         form.save()
         Review.objects.filter(id=review_id).update(editorreview=True)
@@ -98,5 +105,47 @@ def delete(request, addon_id, review_id):
     log.info('DELETE: %s deleted %s by %s ("%s": "%s")' %
              (request.amo_user.display_name, review_id,
               review.user.display_name, review.title, review.body))
-    # TODO: Insert into event log.
     return http.HttpResponse()
+
+
+def _review_details(request, addon, form):
+    d = dict(addon_id=addon.id, user_id=request.user.id,
+             version_id=addon.current_version.id,
+             ip_address=request.META.get('REMOTE_ADDR', ''))
+    d.update(**form.cleaned_data)
+    return d
+
+
+@login_required
+def reply(request, addon_id, review_id):
+    addon = get_object_or_404(Addon.objects.valid(), id=addon_id)
+    is_admin = acl.action_allowed(request, 'Admin', 'EditAnyAddon')
+    is_author = acl.check_ownership(request, addon, require_owner=True)
+    if not is_admin or is_author:
+        return http.HttpResponseForbidden()
+
+    review = get_object_or_404(Review.objects, pk=review_id, addon=addon_id)
+    form = forms.ReviewReplyForm(request.POST or None)
+    if request.method == 'POST':
+        if form.is_valid():
+            r = Review.objects.create(reply_to_id=review_id,
+                                      **_review_details(request, addon, form))
+            log.debug('New reply to %s: %s' % (review_id, r.id))
+            return redirect('reviews.detail', addon_id, review_id)
+    ctx = dict(review=review, form=form, addon=addon)
+    ctx.update(flag_context())
+    return jingo.render(request, 'reviews/reply.html', ctx)
+
+
+@login_required
+def add(request, addon_id):
+    addon = get_object_or_404(Addon.objects.valid(), id=addon_id)
+    form = forms.ReviewForm(request.POST or None)
+    if request.method == 'POST':
+        if form.is_valid():
+            details = _review_details(request, addon, form)
+            review = Review.objects.create(**details)
+            log.debug('New review: %s' % review.id)
+            return redirect('reviews.detail', addon_id, review.id)
+    return jingo.render(request, 'reviews/add.html',
+                        dict(addon=addon, form=form))

media/css/zamboni/zamboni.css

@@ -2178,6 +2178,9 @@ ul.review-options > li:not(:first-child) {
     clear: left;
     font-size: .9em;
 }
+.review.deleted {
+    display: none;
+}
 .reviews .highlight > span {
     display: block;
 }

migrations/58-reviews-null.sql

@@ -0,0 +1,3 @@
+ALTER TABLE reviews
+    CHANGE COLUMN `title` `title` int(11) unsigned NULL,
+    CHANGE COLUMN `body` `body` int(11) unsigned NULL;