Merge pull request #479 from mouse-reeve/books-ui-cleanup

Books UI cleanup

bookwyrm/templates/book.html

@@ -9,6 +9,9 @@
             <h1 class="title">
                 {{ book.title }}{% if book.subtitle %}:
                 <small>{{ book.subtitle }}</small>{% endif %}
+                {% if book.series %}
+                <small class="has-text-grey-dark">({{ book.series }}{% if book.series_number %} #{{ book.series_number }}{% endif %})</small><br>
+                {% endif %}
             </h1>
             {% if book.authors %}
             <h2 class="subtitle">
@@ -49,19 +52,44 @@
             </div>
             {% endif %}
 
-            <dl class="content">
-            {% for field in info_fields %}
-                {% if field.value %}
-                <dt>{{ field.name }}:</dt>
-                <dd>{{ field.value }}</dd>
+            <section class="content">
+                <dl>
+                    {% if book.isbn_13 %}
+                    <div class="is-flex is-justify-content-space-between">
+                        <dt>ISBN:</dt>
+                        <dd>{{ book.isbn_13 }}</dd>
+                    </div>
+                    {% endif %}
+
+                    {% if book.oclc_number %}
+                    <div class="is-flex is-justify-content-space-between">
+                        <dt>OCLC Number:</dt>
+                        <dd>{{ book.oclc_number }}</dd>
+                    </div>
+                    {% endif %}
+
+                    {% if book.asin %}
+                    <div class="is-flex is-justify-content-space-between">
+                        <dt>ASIN:</dt>
+                        <dd>{{ book.asin }}</dd>
+                    </div>
                     {% endif %}
-            {% endfor %}
                 </dl>
+
+                <p>
+                {% if book.physical_format %}{{ book.physical_format | title }}{% if book.pages %},<br>{% endif %}{% endif %}
+                {% if book.pages %}{{ book.pages }} pages{% endif %}
+                </p>
+
+                {% if book.openlibrary_key %}
+                <p><a href="https://openlibrary.org/books/{{ book.openlibrary_key }}" target="_blank" rel="noopener">View on OpenLibrary</a></p>
+                {% endif %}
+            </section>
         </div>
 
         <div class="column">
             <div class="block">
-                <h3 class="field is-grouped">{% include 'snippets/stars.html' with rating=rating %} ({{ reviews|length }} review{{ reviews|length|pluralize }})</h3>
+                <h3 class="field is-grouped">{% include 'snippets/stars.html' with rating=rating %} ({{ review_count }} review{{ reviews|length|pluralize }})</h3>
 
                 {% include 'snippets/trimmed_text.html' with full=book|book_description %}
 
@@ -145,16 +173,32 @@
             </div>
 
         </div>
-    </div>
-</div>
-
-
-{% if not reviews %}
-<div class="block">
-    <p>No reviews yet!</p>
-</div>
+        <div class="column is-narrow">
+            {% if book.subjects %}
+            <section class="content block">
+                <h2 class="title is-5">Subjects</h2>
+                <ul>
+                {% for subject in book.subjects %}
+                    <li>{{ subject }}</li>
+                {% endfor %}
+                </ul>
+            </section>
             {% endif %}
 
+            {% if book.subject_places %}
+            <section class="content block">
+                <h2 class="title is-5">Places</h2>
+                <ul>
+                {% for place in book.subject_placess %}
+                    <li>{{ place }}</li>
+                {% endfor %}
+                </ul>
+            </section>
+            {% endif %}
+        </div>
+    </div>
+</div>
+
 <div class="block">
     {% for review in reviews %}
     <div class="block">
@@ -162,9 +206,9 @@
     </div>
     {% endfor %}
 
-    <div class="block columns">
+    <div class="block is-flex is-flex-wrap-wrap">
     {% for rating in ratings %}
-    <div class="column">
+    <div class="block mr-5">
         <div class="media">
             <div class="media-left">{% include 'snippets/avatar.html' with user=rating.user %}</div>
             <div class="media-content">
@@ -185,6 +229,5 @@
     </div>
 </div>
 
-
 {% endblock %}
 

bookwyrm/templates/edit_book.html

@@ -37,10 +37,6 @@
             {% for error in form.title.errors %}
             <p class="help is-danger">{{ error | escape }}</p>
             {% endfor %}
-            <p class="fields is-grouped"><label class="label" for="id_sort_title">Sort title:</label> {{ form.sort_title }} </p>
-            {% for error in form.sort_title.errors %}
-            <p class="help is-danger">{{ error | escape }}</p>
-            {% endfor %}
             <p class="fields is-grouped"><label class="label" for="id_subtitle">Subtitle:</label> {{ form.subtitle }} </p>
             {% for error in form.subtitle.errors %}
             <p class="help is-danger">{{ error | escape }}</p>
@@ -113,12 +109,12 @@
                 {% for error in form.openlibrary_key.errors %}
                 <p class="help is-danger">{{ error | escape }}</p>
                 {% endfor %}
-                <p class="fields is-grouped"><label class="label" for="id_librarything_key">Librarything key:</label> {{ form.librarything_key }} </p>
-                {% for error in form.librarything_key.errors %}
+                <p class="fields is-grouped"><label class="label" for="id_librarything_key">OCLC Number:</label> {{ form.oclc_number }} </p>
+                {% for error in form.oclc_number.errors %}
                 <p class="help is-danger">{{ error | escape }}</p>
                 {% endfor %}
-                <p class="fields is-grouped"><label class="label" for="id_goodreads_key">Goodreads key:</label> {{ form.goodreads_key }} </p>
-                {% for error in form.goodreads_key.errors %}
+                <p class="fields is-grouped"><label class="label" for="id_asin">ASIN:</label> {{ form.asin }} </p>
+                {% for error in form.ASIN.errors %}
                 <p class="help is-danger">{{ error | escape }}</p>
                 {% endfor %}
             </div>

bookwyrm/templates/snippets/rate_action.html

@@ -1,8 +1,9 @@
 {% load bookwyrm_tags %}
+{% if request.user.is_authenticated %}
 <span class="is-sr-only">Leave a rating</span>
 <div class="field is-grouped stars rate-stars">
     {% for i in '12345'|make_list %}
-    <form name="rate" action="/rate/" method="POST" onsubmit="return rate_stars(event)">
+    <form name="rate" action="/rate/" method="POST">
         {% csrf_token %}
         <input type="hidden" name="user" value="{{ request.user.id }}">
         <input type="hidden" name="book" value="{{ book.id }}">
@@ -14,3 +15,4 @@
     </form>
     {% endfor %}
 </div>
+{% endif %}

bookwyrm/tests/test_views.py

@@ -116,17 +116,20 @@ class Views(TestCase):
             content='blah blah blah', user=rat, privacy='followers')
         rat_mention.mention_users.set([self.local_user])
 
-        statuses = views.get_activity_feed(self.local_user, 'home')
-        self.assertEqual(len(statuses), 2)
-        self.assertEqual(statuses[1], public_status)
-        self.assertEqual(statuses[0], rat_mention)
-
         statuses = views.get_activity_feed(
-            self.local_user, 'home', model=models.Comment)
+            self.local_user,
+            ['public', 'unlisted', 'followers'],
+            following_only=True,
+            queryset=models.Comment.objects
+        )
         self.assertEqual(len(statuses), 1)
         self.assertEqual(statuses[0], public_status)
 
-        statuses = views.get_activity_feed(self.local_user, 'local')
+        statuses = views.get_activity_feed(
+            self.local_user,
+            ['public', 'followers'],
+            local_only=True
+        )
         self.assertEqual(len(statuses), 2)
         self.assertEqual(statuses[1], public_status)
         self.assertEqual(statuses[0], rat_public)
@@ -135,19 +138,30 @@ class Views(TestCase):
         self.assertEqual(len(statuses), 1)
         self.assertEqual(statuses[0], direct_status)
 
-        statuses = views.get_activity_feed(self.local_user, 'federated')
+        statuses = views.get_activity_feed(
+            self.local_user,
+            ['public', 'followers'],
+        )
         self.assertEqual(len(statuses), 3)
         self.assertEqual(statuses[2], public_status)
         self.assertEqual(statuses[1], rat_public)
         self.assertEqual(statuses[0], remote_status)
 
-        statuses = views.get_activity_feed(self.local_user, 'friends')
+        statuses = views.get_activity_feed(
+            self.local_user,
+            ['public', 'unlisted', 'followers'],
+            following_only=True
+        )
         self.assertEqual(len(statuses), 2)
         self.assertEqual(statuses[1], public_status)
         self.assertEqual(statuses[0], rat_mention)
 
         rat.followers.add(self.local_user)
-        statuses = views.get_activity_feed(self.local_user, 'friends')
+        statuses = views.get_activity_feed(
+            self.local_user,
+            ['public', 'unlisted', 'followers'],
+            following_only=True
+        )
         self.assertEqual(len(statuses), 5)
         self.assertEqual(statuses[4], public_status)
         self.assertEqual(statuses[3], rat_public)

bookwyrm/views.py

@@ -83,7 +83,16 @@ def home_tab(request, tab):
 
     suggested_books = get_suggested_books(request.user)
 
-    activities = get_activity_feed(request.user, tab)
+    if tab == 'home':
+        activities = get_activity_feed(
+            request.user, ['public', 'unlisted', 'followers'],
+            following_only=True)
+    elif tab == 'local':
+        activities = get_activity_feed(
+            request.user, ['public', 'followers'], local_only=True)
+    else:
+        activities = get_activity_feed(
+            request.user, ['public', 'followers'])
     paginated = Paginator(activities, PAGE_LENGTH)
     activity_page = paginated.page(page)
 
@@ -151,7 +160,7 @@ def discover_page(request):
             book__in=book.parent_work.editions.all()
         )
         reviews = get_activity_feed(
-            request.user, 'federated', model=reviews)
+            request.user, ['public', 'unlisted'], queryset=reviews)
         ratings[book.id] = reviews.aggregate(Avg('rating'))['rating__avg']
     data = {
         'title': 'Discover',
@@ -187,68 +196,62 @@ def direct_messages_page(request, page=1):
     return TemplateResponse(request, 'direct_messages.html', data)
 
 
-def get_activity_feed(user, filter_level, model=models.Status):
+def get_activity_feed(
+        user, privacy, local_only=False, following_only=False,
+        queryset=models.Status.objects):
     ''' get a filtered queryset of statuses '''
+    privacy = privacy if isinstance(privacy, list) else [privacy]
+    # if we're looking at Status, we need this. We don't if it's Comment
+    if hasattr(queryset, 'select_subclasses'):
+        queryset = queryset.select_subclasses()
+
+    # exclude deleted
+    queryset = queryset.exclude(deleted=True).order_by('-published_date')
+
+    # you can't see followers only or direct messages if you're not logged in
     if user.is_anonymous:
-        user = None
+        privacy = [p for p in privacy if not p in ['followers', 'direct']]
 
-    if user:
-        following = models.User.objects.filter(
-            Q(followers=user) | Q(id=user.id)
+    # filter to only privided privacy levels
+    queryset = queryset.filter(privacy__in=privacy)
+
+    # only include statuses the user follows
+    if following_only:
+        queryset = queryset.exclude(
+            ~Q(# remove everythign except
+                Q(user__in=user.following.all()) | # user follwoing
+                Q(user=user) |# is self
+                Q(mention_users=user)# mentions user
+            ),
         )
-    else:
-        following = []
-
-    activities = model
-    if hasattr(model, 'objects'):
-        activities = model.objects
-
-    activities = activities.filter(
-        deleted=False,
-    ).order_by(
-        '-published_date'
+    # exclude followers-only statuses the user doesn't follow
+    elif 'followers' in privacy:
+        queryset = queryset.exclude(
+            ~Q(# user isn't following and it isn't their own status
+                Q(user__in=user.following.all()) | Q(user=user)
+            ),
+            privacy='followers' # and the status is followers only
         )
 
-    if filter_level == 'direct':
-        return activities.filter(
-            Q(user=user) | Q(mention_users=user),
-            privacy='direct'
-        ).distinct()
-
-    # never show DMs in the regular feed
-    activities = activities.filter(~Q(privacy='direct'))
-
-
-    if hasattr(activities, 'select_subclasses'):
-        activities = activities.select_subclasses()
-
-    if filter_level in ['friends', 'home']:
-        # people you follow and direct mentions
-        activities = activities.filter(
-            Q(user__in=following, privacy__in=[
-                'public', 'unlisted', 'followers'
-            ]) | Q(mention_users=user) | Q(user=user)
-        ).distinct()
-    elif filter_level == 'self':
-        activities = activities.filter(user=user, privacy='public')
-    elif filter_level == 'local':
-        # everyone on this instance except unlisted
-        activities = activities.filter(
-            Q(user__in=following, privacy='followers') | Q(privacy='public'),
-            user__local=True
-        )
-    else:
-        # all activities from everyone you federate with
-        activities = activities.filter(
-            Q(user__in=following, privacy='followers') | Q(privacy='public')
+    # exclude direct messages not intended for the user
+    if 'direct' in privacy:
+        queryset = queryset.exclude(
+            ~Q(
+                Q(user=user) | Q(mention_users=user)
+            ), privacy='direct'
         )
 
+    # filter for only local status
+    if local_only:
+        queryset = queryset.filter(user__local=True)
+
+    # remove statuses that have boosts in the same queryset
     try:
-        activities = activities.filter(~Q(boosters__in=activities))
+        queryset = queryset.filter(~Q(boosters__in=queryset))
     except ValueError:
         pass
 
-    return activities
+    return queryset
 
 
 @require_GET
@@ -462,7 +465,11 @@ def user_page(request, username):
             break
 
     # user's posts
-    activities = get_activity_feed(user, 'self')
+    activities = get_activity_feed(
+        user,
+        ['public', 'unlisted', 'followers'],
+        queryset=models.Status.objects.filter(user=request.user)
+    )
     paginated = Paginator(activities, PAGE_LENGTH)
     activity_page = paginated.page(page)
 
@@ -629,10 +636,16 @@ def book_page(request, book_id):
         book__in=work.editions.all(),
     )
     # all reviews for the book
-    reviews = get_activity_feed(request.user, 'federated', model=reviews)
+    reviews = get_activity_feed(
+        request.user,
+        ['public', 'unlisted', 'followers', 'direct'],
+        queryset=reviews
+    )
 
     # the reviews to show
-    paginated = Paginator(reviews.filter(content__isnull=False), PAGE_LENGTH)
+    paginated = Paginator(reviews.exclude(
+        Q(content__isnull=True) | Q(content='')
+    ), PAGE_LENGTH)
     reviews_page = paginated.page(page)
 
     prev_page = next_page = None
@@ -668,7 +681,8 @@ def book_page(request, book_id):
         'title': book.title,
         'book': book,
         'reviews': reviews_page,
-        'ratings': reviews.filter(content__isnull=True),
+        'review_count': reviews.count(),
+        'ratings': reviews.filter(Q(content__isnull=True) | Q(content='')),
         'rating': reviews.aggregate(Avg('rating'))['rating__avg'],
         'tags':  models.UserTag.objects.filter(book=book),
         'user_tags': user_tags,
@@ -676,14 +690,6 @@ def book_page(request, book_id):
         'other_edition_shelves': other_edition_shelves,
         'readthroughs': readthroughs,
         'path': '/book/%s' % book_id,
-        'info_fields': [
-            {'name': 'ISBN', 'value': book.isbn_13},
-            {'name': 'OCLC number', 'value': book.oclc_number},
-            {'name': 'OpenLibrary ID', 'value': book.openlibrary_key},
-            {'name': 'Goodreads ID', 'value': book.goodreads_key},
-            {'name': 'Format', 'value': book.physical_format},
-            {'name': 'Pages', 'value': book.pages},
-        ],
         'next': next_page,
         'prev': prev_page,
     }