Merge pull request #393 from metabrainz/minor-fixes

Some misc minor fixes

critiquebrainz/db/review.py

@@ -525,8 +525,8 @@ def list_reviews(*, inc_drafts=False, inc_hidden=False, entity_id=None, entity_t
         inc_drafts (bool): True if reviews marked as drafts should be included, False if not.
         inc_hidden (bool): True if reviews marked as hidden should be included, False if not.
         exclude (list): List of reviews (their IDs) to exclude from results.
-        review_type (str): Filter reviews. Can either be "review"(for reviews with only text), or "rating" (for
-                           reviews with only rating), or "None" (for all reviews).
+        review_type (str): Return reviews of this type. Can either be "review" (to return reviews with text),
+                           or "rating" (to return reviews which have a rating), or ``None`` (to return all reviews).
 
     Returns:
         Tuple with two values:

critiquebrainz/frontend/templates/macros.html

@@ -225,28 +225,37 @@ <h4 style="margin-bottom:0;">{{ _('Rate this {}:'.format(entity_type_readable))
   {% set total_pages = (count/limit)|round(method='ceil')|int %}
   {% if count > limit %}
     <div class="col-md-12 text-center">
-      <nav aria-label="Page navigation">
+      <nav aria-label="{{ _('Page Navigation') }}">
         <ul class="pagination">
+          <li class="previous {{'disabled' if current_page <= 1 }}">
+              <a aria-label="{{ _('Previous') }}" href="{{ url_for(page_info, page=page-1, **parameter) }}">&larr; {{ _('Previous') }}</a>
+          </li>
 
-          <li class="previous {{'disabled' if current_page <= 1 }}"><a aria-label="Previous" href="{{ url_for(page_info, page=page-1, **parameter) }}">&larr; {{ _('Previous') }}</a></li>
-
-          <li {{ "class=active" if current_page == 1 }}><a href="{{ url_for(page_info, page=1,  **parameter) }}">{{ 1 }}</a></li>
+          <li {{ "class=active" if current_page == 1 }}>
+              <a href="{{ url_for(page_info, page=1,  **parameter) }}">{{ 1 }}</a>
+          </li>
 
           {% if current_page >= 5 %}
             <li class="disabled"><a>{{ '...' }}</a></li>
           {% endif %}
 
           {% for page_index in range([2,current_page-2]|max, [total_pages,current_page+3]|min) %}
-            <li {{ "class=active" if current_page == page_index }}><a href="{{ url_for(page_info, page=page_index, **parameter) }}">{{ page_index }}</a></li>
+            <li {{ "class=active" if current_page == page_index }}>
+                <a href="{{ url_for(page_info, page=page_index, **parameter) }}">{{ page_index }}</a>
+            </li>
           {% endfor %}
 
           {% if (current_page <= (total_pages - 4)) %}
             <li class='disabled'><a>{{ '...' }}</a></li>
           {% endif %}
 
-          <li {{ "class=active" if current_page == total_pages }}><a href="{{ url_for(page_info, page=total_pages, **parameter) }}">{{ total_pages }}</a></li>
+          <li {{ "class=active" if current_page == total_pages }}>
+              <a href="{{ url_for(page_info, page=total_pages, **parameter) }}">{{ total_pages }}</a>
+          </li>
 
-          <li class="next {{'disabled' if current_page == total_pages }}"><a aria-label="Next" href="{{ url_for(page_info, page=page+1, **parameter) }}">{{ _('Next') }} &rarr;</a></li>
+          <li class="next {{'disabled' if current_page == total_pages }}">
+              <a aria-label="{{ _('Next') }}" href="{{ url_for(page_info, page=page+1, **parameter) }}">{{ _('Next') }} &rarr;</a>
+          </li>
         </ul>
       </nav>
     </div>

critiquebrainz/frontend/templates/user/reviews.html

@@ -16,7 +16,7 @@
         <th>{{ _('Created on') }}</th>
         <th>{{ _('Published on')}} </th>
         <th>{{ _('Votes (+/-)') }}</th>
-		  <th>{{ _('Rating') }}</th>
+		<th>{{ _('Rating') }}</th>
         {% if current_user.is_authenticated and current_user == user %}
           <th><!-- Buttons --></th>
         {% endif %}

critiquebrainz/ws/review/test/views_test.py

@@ -112,6 +112,10 @@ def test_review_type(self):
         self.assertCountEqual(actual_review_ids, expected_review_ids)
 
     def test_review_large_count(self):
+        """Test that retrieving reviews of a particular type correctly returns the total number of
+        reviews of this type in addition to the paged results"""
+
+        # 100 text reviews and 1 rating
         for _ in range(100):
             review = dict(
                 entity_id=uuid.uuid4(),
@@ -122,6 +126,19 @@ def test_review_large_count(self):
                 license_id=self.license["id"],
             )
             db_review.create(**review)
+
+        db_review.create(
+            entity_id="2b3abc25-7453-39f3-86c4-1441f360e121",
+            entity_type='release_group',
+            user_id=self.user.id,
+            rating=5,
+            is_draft=False,
+            license_id=self.license["id"],
+        )
+        resp = self.client.get('/review/')
+        self.assert200(resp)
+        self.assertEqual(resp.json["count"], 101)
+
         resp = self.client.get('/review/', query_string={'review_type': 'review'})
         self.assert200(resp)
         self.assertEqual(resp.json["count"], 100)

critiquebrainz/ws/review/views.py

@@ -335,14 +335,14 @@ def review_list_handler():
           ]
         }
 
-    :json uuid entity_id: UUID of the entity that is being reviewed
-    :json string entity_type: One of the supported reviewable entities. :data:`critiquebrainz.db.review.ENTITY_TYPES` **(optional)**
+    :query entity_id: UUID of an entity to retrieve reviews for **(optional)**
+    :query entity_type: One of the supported reviewable entities. :data:`critiquebrainz.db.review.ENTITY_TYPES` **(optional)**
     :query user_id: user's UUID **(optional)**
     :query sort: ``popularity`` or ``published_on`` **(optional)**
     :query limit: results limit, min is 0, max is 50, default is 50 **(optional)**
     :query offset: result offset, default is 0 **(optional)**
     :query language: language code (ISO 639-1) **(optional)**
-    :query review_type: ``review`` or ``rating`` **(optional)**
+    :query review_type: ``review`` or ``rating``. If set, only return reviews which have a text review, or a rating **(optional)**
 
     :resheader Content-Type: *application/json*
     """