Merge pull request #1692 from joachimesque/summary-review-sharing

Feature: Annual summary sharing

bookwyrm/migrations/0121_user_summary_keys.py

@@ -0,0 +1,18 @@
+# Generated by Django 3.2.5 on 2021-12-22 11:06
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("bookwyrm", "0120_list_embed_key"),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name="user",
+            name="summary_keys",
+            field=models.JSONField(null=True),
+        ),
+    ]

bookwyrm/models/user.py

@@ -148,6 +148,8 @@ class User(OrderedCollectionPageMixin, AbstractUser):
         size=8,
         default=get_feed_filter_choices,
     )
+    # annual summary keys
+    summary_keys = models.JSONField(null=True)
 
     preferred_timezone = models.CharField(
         choices=[(str(tz), str(tz)) for tz in pytz.all_timezones],

bookwyrm/static/css/bookwyrm.css

@@ -125,6 +125,10 @@ input[type=file]::file-selector-button:hover {
 /** General `details` element styles
  ******************************************************************************/
 
+summary {
+    cursor: pointer;
+}
+
 summary::-webkit-details-marker {
     display: none;
 }
@@ -634,6 +638,37 @@ ol.ordered-list li::before {
     min-height: calc(2 * var(--height-basis));
 }
 
+/* Copy
+ ******************************************************************************/
+
+.horizontal-copy {
+    display: flex;
+    flex-direction: row;
+    align-items: center;
+    gap: 0.75rem;
+}
+
+.horizontal-copy textarea {
+    min-width: initial;
+    white-space: nowrap;
+}
+
+.horizontal-copy button {
+    align-self: stretch;
+    height: unset;
+}
+
+.vertical-copy {
+    display: flex;
+    flex-direction: column;
+    align-items: stretch;
+    gap: 0.75rem;
+}
+
+.vertical-copy button {
+    width: 100%;
+}
+
 /* Dimensions
  * @todo These could be in rem.
  ******************************************************************************/

bookwyrm/static/js/bookwyrm.js

@@ -471,10 +471,8 @@ let BookWyrm = new class {
 
         copyButtonEl.textContent = textareaEl.dataset.copytextLabel;
         copyButtonEl.classList.add(
-            "mt-2",
             "button",
             "is-small",
-            "is-fullwidth",
             "is-primary",
             "is-light"
         );

bookwyrm/templates/annual_summary/layout.html

@@ -10,42 +10,103 @@
 {% endblock %}
 
 {% block content %}
+{% with display_name=summary_user.display_name %}
+{% if user == summary_user %}
     <div class="columns">
     {% with year=paginated_years|first %}
+    {% if year %}
         <div class="column">
-            <a href="{% url 'annual-summary' year %}">
+            <a href="{% url 'annual-summary' summary_user.localname year %}">
                 <span class="icon icon-arrow-left" aria-hidden="true"></span>
-            {% blocktrans %}{{ year }} in the books{% endblocktrans %}
+                {{ year }}
             </a>
         </div>
+    {% endif %}
     {% endwith %}
 
     {% with year=paginated_years|last %}
     {% if year %}
         <div class="column has-text-right">
-            <a href="{% url 'annual-summary' year %}">
+            <a href="{% url 'annual-summary' summary_user.localname year %}">
-            {% blocktrans %}{{ year }} in the books{% endblocktrans %}
+                {{ year }}
                 <span class="icon icon-arrow-right" aria-hidden="true"></span>
             </a>
         </div>
     {% endif %}
     {% endwith %}
     </div>
+{% endif %}
 
-    <h1 class="title is-1 is-serif has-text-centered mb-5">
+    <h1 class="title is-1 is-serif has-text-centered">
         📚✨
         {% blocktrans %}{{ year }} <em>in the books</em>{% endblocktrans %}
         ✨📚
     </h1>
+    <p class="subtitle is-3 is-serif has-text-centered mb-5">
+        {% blocktrans %}<em>{{ display_name }}’s</em> year of reading{% endblocktrans %}
+    </p>
 
-    {% if not books %}
+    <details>
-        <p class="has-text-centered is-size-5">{% blocktrans %}Sadly you didn't finish any book in {{ year }}{% endblocktrans %}</p>
+        <summary class="has-text-centered">
-    {% else %}
+            <span role="heading" aria-level="2" class="title is-6 has-text-success-dark">
+                {% trans "Share this page" %}
+            </span>
+        </summary>
+        <div class="columns mt-3">
+            <div class="column is-three-fifths is-offset-one-fifth">
+
+            {% if year_key %}
+                <div class="horizontal-copy mb-5">
+                    <textarea rows="1" readonly class="textarea is-small" aria-labelledby="embed-label" data-copytext data-copytext-label="{% trans 'Copy address' %}" data-copytext-success="{% trans 'Copied!' %}">{{ request.scheme|add:"://"|add:request.get_host|add:request.path }}?key={{ year_key }}</textarea>
+                </div>
+            {% endif %}
+
+            {% if user == summary_user %}
+            {% if year_key %}
+                <div class="columns mb-2">
+                    <div class="column pb-0">
+                        <p>{% trans "Sharing status: <strong>public with key</strong>" %}</p>
+                        <p>{% trans "The page can be seen by anyone with the complete address." %}</p>
+                    </div>
+                    <form class="column pb-0 is-narrow" method="post" action="{% url "summary-revoke-key" %}" id="revoke-key">
+                        {% csrf_token %}
+                        <input type="hidden" name="year" value="{{ year }}" />
+                        <button class="button is-danger is-outlined" type="submit">{% trans "Make page private" %}</button>
+                    </form>
+                </div>
+            {% else %}
+                <div class="columns">
+                    <div class="column pb-0">
+                        <p>{% trans "Sharing status: <strong>private</strong>" %}</p>
+                        <p>{% trans "The page is private, only you can see it." %}</p>
+                    </div>
+                    <form class="column pb-0 is-narrow" method="post" action="{% url "summary-add-key" %}" id="add-key">
+                        {% csrf_token %}
+                        <input type="hidden" name="year" value="{{ year }}" />
+                        <button class="button is-primary is-outlined" type="submit">{% trans "Make page public" %}</button>
+                    </form>
+                </div>
+            {% endif %}
+                <p class="help">{% trans "When you make your page private, the old key won’t give access to the page anymore. A new key will be created if the page is once again made public." %}</p>
+            {% endif %}
+            </div>
+        </div>
+    </details>
+
+    <div class="columns mt-1">
+        <div class="column is-one-fifth is-offset-two-fifths">
+            <hr />
+        </div>
+    </div>
+
+{% if not books %}
+    <p class="has-text-centered is-size-5">{% blocktrans %}Sadly {{ display_name }} didn’t finish any book in {{ year }}{% endblocktrans %}</p>
+{% else %}
 
     <div class="columns is-mobile">
         <div class="column is-8 is-offset-2 has-text-centered">
             <h2 class="title is-3 is-serif">
-                {% blocktrans %}In {{ year }}, you read {{ books_total }} books<br />for a total of {{ pages_total }} pages!{% endblocktrans %}
+                {% blocktrans %}In {{ year }}, {{ display_name }} read {{ books_total }} books<br />for a total of {{ pages_total }} pages!{% endblocktrans %}
             </h2>
             <p class="subtitle is-5">{% trans "That’s great!" %}</p>
 
@@ -71,7 +132,7 @@
             <a href="{{ book_pages_lowest.local_path }}">{% include 'snippets/book_cover.html' with book=book_pages_lowest cover_class='is-w-auto-tablet is-h-l-mobile' %}</a>
         </div>
         <div class="column is-3">
-            {% trans "Your shortest read this year" %}
+            {% trans "Their shortest read this year…" %}
             <p class="title is-4 is-serif is-italic">
                 <a href="{{ book_pages_lowest.local_path }}" class="has-text-success-dark">
                     {{ book_pages_lowest.title }}
@@ -92,7 +153,7 @@
             <a href="{{ book_pages_highest.local_path }}">{% include 'snippets/book_cover.html' with book=book_pages_highest cover_class='is-w-auto-tablet is-h-l-mobile' %}</a>
         </div>
         <div class="column is-3">
-            {% trans "and the longest read" %}
+            {% trans "…and the longest" %}
             <p class="title is-4 is-serif is-italic">
                 <a href="{{ book_pages_lowest.local_path }}" class="has-text-success-dark">
                     {{ book_pages_highest.title }}
@@ -118,10 +179,11 @@
         </div>
     </div>
 
+    {% if ratings_total > 0 %}
     <div class="columns">
         <div class="column has-text-centered">
             <h2 class="title is-3 is-serif">
-            {% blocktrans %}You left {{ ratings_total }} ratings, <br />your average rating is {{ rating_average }}{% endblocktrans %}
+            {% blocktrans %}{{ display_name }} left {{ ratings_total }} ratings, <br />their average rating is {{ rating_average }}{% endblocktrans %}
             </h2>
         </div>
     </div>
@@ -131,7 +193,7 @@
         </div>
         {% if book_rating_highest %}
         <div class="column is-4">
-            {% trans "Your best rated review" %}
+            {% trans "Their best rated review" %}
             <p class="title is-4 is-serif is-italic">
                 <a href="{{ book_rating_highest.book.local_path }}" class="has-text-success-dark">
                     {{ book_rating_highest.book.title }}
@@ -144,7 +206,7 @@
             {% endif %}
             <p class="subtitle is-6">
                 {% with rating=book_rating_highest.rating|floatformat %}
-                {% blocktrans %}Your rating: <strong>{{ rating }}</strong>{% endblocktrans%}
+                {% blocktrans %}Their rating: <strong>{{ rating }}</strong>{% endblocktrans%}
                 {% endwith %}
             </p>
         </div>
@@ -156,11 +218,12 @@
             <hr />
         </div>
     </div>
+    {% endif %}
 
     <div class="columns">
         <div class="column has-text-centered">
             <h2 class="title is-3 is-serif">
-            {% blocktrans %}All the books you read in 2021{% endblocktrans %}
+            {% blocktrans %}All the books {{ display_name }} read in 2021{% endblocktrans %}
             </h2>
         </div>
     </div>
@@ -188,5 +251,6 @@
             </div>
         </div>
     </div>
-    {% endif %}
+{% endif %}
+{% endwith %}
 {% endblock %}

bookwyrm/templates/feed/summary_card.html

@@ -15,7 +15,7 @@
 </p>
 
 <p>
-    <a href="{% url 'annual-summary' year %}" class="button is-success has-background-success-dark">
+    <a href="{% url 'annual-summary' request.user.localname year %}" class="button is-success has-background-success-dark">
     {% blocktrans %}Discover your stats for {{ year }}!{% endblocktrans %}
     </a>
 </p>

bookwyrm/templates/lists/list.html

@@ -190,7 +190,9 @@
             <h2 class="title is-5 mt-6" id="embed-label">
                 {% trans "Embed this list on a website" %}
             </h2>
-            <textarea readonly class="textarea is-small" aria-labelledby="embed-label" data-copytext data-copytext-label="{% trans 'Copy embed code' %}" data-copytext-success="{% trans 'Copied!' %}"><iframe style="border-width:0;" id="bookwyrm_list_embed" width="400" height="600" title="{% blocktrans with list_name=list.name site_name=site.name owner=list.user.display_name %}{{ list_name }}, a list by {{owner}} on {{ site_name }}{% endblocktrans %}" src="{{ embed_url }}"></iframe></textarea>
+            <div class="vertical-copy">
+                <textarea readonly class="textarea is-small" aria-labelledby="embed-label" data-copytext data-copytext-label="{% trans 'Copy embed code' %}" data-copytext-success="{% trans 'Copied!' %}"><iframe style="border-width:0;" id="bookwyrm_list_embed" width="400" height="600" title="{% blocktrans with list_name=list.name site_name=site.name owner=list.user.display_name %}{{ list_name }}, a list by {{owner}} on {{ site_name }}{% endblocktrans %}" src="{{ embed_url }}"></iframe></textarea>
+            </div>
         </div>
 
     </section>

bookwyrm/tests/views/test_annual_summary.py

@@ -34,6 +34,7 @@ class AnnualSummary(TestCase):
                 local=True,
                 localname="mouse",
                 remote_id="https://example.com/users/mouse",
+                summary_keys={"2020": "0123456789"},
             )
         self.work = models.Work.objects.create(title="Test Work")
         self.book = models.Edition.objects.create(
@@ -42,18 +43,11 @@ class AnnualSummary(TestCase):
             parent_work=self.work,
             pages=300,
         )
-        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"):
+
-            self.review = models.Review.objects.create(
-                name="Review name",
-                content="test content",
-                rating=3.0,
-                user=self.local_user,
-                book=self.book,
-            )
         self.anonymous_user = AnonymousUser
         self.anonymous_user.is_authenticated = False
 
-        self.year = 2020
+        self.year = "2020"
         models.SiteSettings.objects.create()
 
     def test_annual_summary_not_authenticated(self, *_):
@@ -62,12 +56,24 @@ class AnnualSummary(TestCase):
         request = self.factory.get("")
         request.user = self.anonymous_user
 
-        with patch(
+        with self.assertRaises(Http404):
-            "bookwyrm.views.annual_summary.is_year_available"
+            view(request, self.local_user.localname, self.year)
-        ) as is_year_available:
+
-            is_year_available.return_value = True
+    def test_annual_summary_not_authenticated_with_key(self, *_):
-            with self.assertRaises(Http404):
+        """there are so many views, this just makes sure it DOES LOAD"""
-                view(request, self.year)
+        key = self.local_user.summary_keys[self.year]
+        view = views.AnnualSummary.as_view()
+        request_url = (
+            f"user/{self.local_user.localname}/{self.year}-in-the-books?key={key}"
+        )
+        request = self.factory.get(request_url)
+        request.user = self.anonymous_user
+
+        result = view(request, self.local_user.localname, self.year)
+
+        self.assertIsInstance(result, TemplateResponse)
+        validate_html(result.render())
+        self.assertEqual(result.status_code, 200)
 
     def test_annual_summary_wrong_year(self, *_):
         """there are so many views, this just makes sure it DOESN’T LOAD"""
@@ -75,12 +81,8 @@ class AnnualSummary(TestCase):
         request = self.factory.get("")
         request.user = self.anonymous_user
 
-        with patch(
+        with self.assertRaises(Http404):
-            "bookwyrm.views.annual_summary.is_year_available"
+            view(request, self.local_user.localname, self.year)
-        ) as is_year_available:
-            is_year_available.return_value = False
-            with self.assertRaises(Http404):
-                view(request, self.year)
 
     def test_annual_summary_empty_page(self, *_):
         """there are so many views, this just makes sure it LOADS"""
@@ -88,11 +90,8 @@ class AnnualSummary(TestCase):
         request = self.factory.get("")
         request.user = self.local_user
 
-        with patch(
+        result = view(request, self.local_user.localname, self.year)
-            "bookwyrm.views.annual_summary.is_year_available"
+
-        ) as is_year_available:
-            is_year_available.return_value = True
-            result = view(request, self.year)
         self.assertIsInstance(result, TemplateResponse)
         validate_html(result.render())
         self.assertEqual(result.status_code, 200)
@@ -114,11 +113,38 @@ class AnnualSummary(TestCase):
         request = self.factory.get("")
         request.user = self.local_user
 
-        with patch(
+        result = view(request, self.local_user.localname, self.year)
-            "bookwyrm.views.annual_summary.is_year_available"
+
-        ) as is_year_available:
+        self.assertIsInstance(result, TemplateResponse)
-            is_year_available.return_value = True
+        validate_html(result.render())
-            result = view(request, self.year)
+        self.assertEqual(result.status_code, 200)
+
+    @patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async")
+    @patch("bookwyrm.activitystreams.add_book_statuses_task.delay")
+    def test_annual_summary_page_with_review(self, *_):
+        """there are so many views, this just makes sure it LOADS"""
+
+        self.review = models.Review.objects.create(
+            name="Review name",
+            content="test content",
+            rating=3.0,
+            user=self.local_user,
+            book=self.book,
+        )
+
+        shelf = self.local_user.shelf_set.filter(identifier="read").first()
+        models.ShelfBook.objects.create(
+            book=self.book,
+            user=self.local_user,
+            shelf=shelf,
+            shelved_date=make_date(2020, 1, 1),
+        )
+
+        view = views.AnnualSummary.as_view()
+        request = self.factory.get("")
+        request.user = self.local_user
+
+        result = view(request, self.local_user.localname, self.year)
 
         self.assertIsInstance(result, TemplateResponse)
         validate_html(result.render())

bookwyrm/urls.py

@@ -479,8 +479,16 @@ urlpatterns = [
     ),
     # annual summary
     re_path(
-        r"^my-year-in-the-books/(?P<year>\d{4})/?$",
+        r"^my-year-in-the-books/(?P<year>\d+)/?$",
+        views.personal_annual_summary,
+    ),
+    re_path(
+        rf"{LOCAL_USER_PATH}/(?P<year>\d+)-in-the-books/?$",
         views.AnnualSummary.as_view(),
         name="annual-summary",
     ),
+    re_path(r"^summary_add_key/?$", views.summary_add_key, name="summary-add-key"),
+    re_path(
+        r"^summary_revoke_key/?$", views.summary_revoke_key, name="summary-revoke-key"
+    ),
 ] + static(settings.MEDIA_URL, document_root=settings.MEDIA_ROOT)

bookwyrm/views/__init__.py

@@ -96,4 +96,9 @@ from .status import edit_readthrough
 from .updates import get_notification_count, get_unread_status_count
 from .user import User, Followers, Following, hide_suggestions
 from .wellknown import *
-from .annual_summary import AnnualSummary
+from .annual_summary import (
+    AnnualSummary,
+    personal_annual_summary,
+    summary_add_key,
+    summary_revoke_key,
+)

bookwyrm/views/annual_summary.py

@@ -1,13 +1,17 @@
 """end-of-year read books stats"""
 from datetime import date
+from uuid import uuid4
 
+from django.contrib.auth.decorators import login_required
 from django.db.models import Case, When, Avg, Sum
 from django.http import Http404
-from django.shortcuts import get_object_or_404
+from django.shortcuts import get_object_or_404, redirect
 from django.template.response import TemplateResponse
 from django.views import View
+from django.views.decorators.http import require_POST
 
 from bookwyrm import models
+from .helpers import get_user_from_username
 
 
 # December day of first availability
@@ -16,59 +20,34 @@ FIRST_DAY = 15
 LAST_DAY = 15
 
 
-def get_annual_summary_year():
-    """return the latest available annual summary year or None"""
-
-    today = date.today()
-    if date(today.year, 12, FIRST_DAY) <= today <= date(today.year, 12, 31):
-        return today.year
-
-    if LAST_DAY > 0 and date(today.year, 1, 1) <= today <= date(
-        today.year, 1, LAST_DAY
-    ):
-        return today.year - 1
-
-    return None
-
-
-def is_year_available(year):
-    """return boolean"""
-
-    today = date.today()
-    year = int(year)
-    if year < today.year:
-        return True
-    if year == today.year and today >= date(today.year, 12, FIRST_DAY):
-        return True
-
-    return False
-
-
 # pylint: disable= no-self-use
 class AnnualSummary(View):
     """display a summary of the year for the current user"""
 
-    def get(self, request, year):
+    def get(self, request, username, year):
         """get response"""
 
-        if not is_year_available(year):
+        user = get_user_from_username(request.user, username)
-            raise Http404(f"The summary for {year} is unavailable")
+
+        year_key = None
+        if user.summary_keys and year in user.summary_keys:
+            year_key = user.summary_keys[year]
+
+        privacy_verification(request, user, year, year_key)
 
         paginated_years = (
-            int(year) - 1,
+            int(year) - 1 if is_year_available(user, int(year) - 1) else None,
-            int(year) + 1 if is_year_available(int(year) + 1) else None,
+            int(year) + 1 if is_year_available(user, int(year) + 1) else None,
         )
 
-        user = request.user
+        # get data
-
-        if not user.is_authenticated:
-            raise Http404(f"Login or register {year} to access this page")
-
         read_book_ids_in_year = get_read_book_ids_in_year(user, year)
 
         if len(read_book_ids_in_year) == 0:
             data = {
+                "summary_user": user,
                 "year": year,
+                "year_key": year_key,
                 "book_total": 0,
                 "books": [],
                 "paginated_years": paginated_years,
@@ -94,7 +73,9 @@ class AnnualSummary(View):
         ratings_stats = ratings.aggregate(Avg("rating"))
 
         data = {
+            "summary_user": user,
             "year": year,
+            "year_key": year_key,
             "books_total": len(read_books_in_year),
             "books": read_books_in_year,
             "pages_total": page_stats["pages__sum"] or 0,
@@ -118,18 +99,166 @@ class AnnualSummary(View):
         return TemplateResponse(request, "annual_summary/layout.html", data)
 
 
+@login_required
+def personal_annual_summary(request, year):
+    """redirect simple URL to URL with username"""
+
+    return redirect("annual-summary", request.user.localname, year)
+
+
+@login_required
+@require_POST
+def summary_add_key(request):
+    """add summary key"""
+
+    year = request.POST["year"]
+    user = request.user
+
+    new_key = uuid4().hex
+
+    if not user.summary_keys:
+        user.summary_keys = {
+            year: new_key,
+        }
+    else:
+        user.summary_keys[year] = new_key
+
+    user.save()
+
+    response = redirect("annual-summary", user.localname, year)
+    response["Location"] += f"?key={str(new_key)}"
+    return response
+
+
+@login_required
+@require_POST
+def summary_revoke_key(request):
+    """revoke summary key"""
+
+    year = request.POST["year"]
+    user = request.user
+
+    if user.summary_keys and year in user.summary_keys:
+        user.summary_keys.pop(year)
+
+    user.save()
+
+    return redirect("annual-summary", user.localname, year)
+
+
+def get_annual_summary_year():
+    """return the latest available annual summary year or None"""
+
+    today = date.today()
+    if date(today.year, 12, FIRST_DAY) <= today <= date(today.year, 12, 31):
+        return today.year
+
+    if LAST_DAY > 0 and date(today.year, 1, 1) <= today <= date(
+        today.year, 1, LAST_DAY
+    ):
+        return today.year - 1
+
+    return None
+
+
+def privacy_verification(request, user, year, year_key):
+    """raises a 404 error if the user should not access the page"""
+    if user != request.user:
+        request_key = None
+        if "key" in request.GET:
+            request_key = request.GET["key"]
+
+        if not request_key or request_key != year_key:
+            raise Http404(f"The summary for {year} is unavailable")
+
+    if not is_year_available(user, year):
+        raise Http404(f"The summary for {year} is unavailable")
+
+
+def is_year_available(user, year):
+    """return boolean"""
+
+    earliest_year = int(get_earliest_year(user, year))
+    today = date.today()
+    year = int(year)
+    if earliest_year <= year < today.year:
+        return True
+    if year == today.year and today >= date(today.year, 12, FIRST_DAY):
+        return True
+
+    return False
+
+
+def get_earliest_year(user, year):
+    """return the earliest finish_date or shelved_date year for user books in read shelf"""
+
+    read_shelfbooks = models.ShelfBook.objects.filter(user__id=user.id).filter(
+        shelf__identifier__exact="read"
+    )
+    read_shelfbooks_list = list(read_shelfbooks.values("book", "shelved_date"))
+
+    book_dates = []
+
+    for book in read_shelfbooks_list:
+        earliest_finished = (
+            models.ReadThrough.objects.filter(user__id=user.id)
+            .filter(book_id=book["book"])
+            .exclude(finish_date__exact=None)
+            .order_by("finish_date")
+            .values("finish_date")
+            .first()
+        )
+
+        if earliest_finished:
+            book_dates.append(
+                min(earliest_finished["finish_date"], book["shelved_date"])
+            )
+        else:
+            book_dates.append(book["shelved_date"])
+
+    if book_dates:
+        return min(book_dates).year
+
+    return year
+
+
 def get_read_book_ids_in_year(user, year):
     """return an ordered QuerySet of the read book ids"""
 
     read_shelf = get_object_or_404(user.shelf_set, identifier="read")
-    read_book_ids_in_year = (
+    shelved_book_ids = (
         models.ShelfBook.objects.filter(shelf=read_shelf)
         .filter(user=user)
-        .filter(shelved_date__year=year)
+        .values_list("book", "shelved_date")
-        .order_by("shelved_date", "created_date", "updated_date")
-        .values_list("book", flat=True)
     )
-    return read_book_ids_in_year
+
+    book_dates = []
+
+    for book in shelved_book_ids:
+        finished_in_year = (
+            models.ReadThrough.objects.filter(user__id=user.id)
+            .filter(book_id=book[0])
+            .filter(finish_date__year=year)
+            .values("finish_date")
+            .first()
+        )
+
+        if finished_in_year:
+            # Finished a readthrough in the year
+            book_dates.append((book[0], finished_in_year["finish_date"]))
+        else:
+            has_other_year_readthrough = (
+                models.ReadThrough.objects.filter(user__id=user.id)
+                .filter(book_id=book[0])
+                .exists()
+            )
+            if not has_other_year_readthrough and book[1].year == int(year):
+                # No readthrough but shelved this year
+                book_dates.append(book)
+
+    book_dates = sorted(book_dates, key=lambda tup: tup[1])
+
+    return [book[0] for book in book_dates]
 
 
 def get_books_from_shelfbooks(books_ids):