Fix lingering weirdness of "upcoming trips"

Stop linking to a permanent redirect straight from the homepage!

We *used* to have different trips pages -- one just for upcoming trips,
and one for all past trips. We did away with `/trips/all` when we had
*thousands* of trips to display.

We left some weird artifacts around -- a pointless class hierarchy, a
misleadingly-named route, and bad URLs. Let's fix all those.

ws/feeds.py

@@ -1,4 +1,7 @@
+from datetime import datetime
+
 from django.contrib.syndication.views import Feed
+from django.db.models import QuerySet
 from django.urls import reverse, reverse_lazy
 from django.utils import timezone
 
@@ -10,27 +13,26 @@
 
 class UpcomingTripsFeed(Feed):
     title = "MITOC Trips"
-    link = reverse_lazy("upcoming_trips")
+    link = reverse_lazy("trips")
     description = "Upcoming trips by the MIT Outing Club"
 
-    def items(self):
-        upcoming_trips = Trip.objects.filter(trip_date__gte=local_date())
-        return upcoming_trips.order_by("-trip_date")
+    def items(self) -> QuerySet[Trip]:
+        return Trip.objects.filter(trip_date__gte=local_date()).order_by("-trip_date")
 
-    def item_title(self, item):
+    def item_title(self, item: Trip) -> str:
         return item.name
 
-    def item_description(self, item):
+    def item_description(self, item: Trip) -> str:
         return item.description
 
-    def item_link(self, item):
+    def item_link(self, item: Trip) -> str:
         return reverse("view_trip", args=[item.pk])
 
-    def item_pubdate(self, item):
+    def item_pubdate(self, item: Trip) -> datetime:
         return item.time_created.astimezone(DEFAULT_TIMEZONE)
 
-    def item_author_name(self, item):
+    def item_author_name(self, item: Trip) -> str:
         return item.creator.name
 
-    def item_updateddate(self, item):
+    def item_updateddate(self, item: Trip) -> datetime:
         return item.last_edited.astimezone(DEFAULT_TIMEZONE)

ws/models.py

@@ -1040,6 +1040,11 @@ def __str__(self) -> str:
 
 
 class Trip(models.Model):
+    # Constant to control the default "page size" when viewing old trips.
+    #
+    # Putting this on the model so the business logic has one source of truth.
+    TRIPS_LOOKBACK = timedelta(days=365)
+
     # When ordering trips which need approval, apply consistent ordering
     # (Defined here to keep the table's default ordering in sync with prev/next buttons
     ordering_for_approval: tuple[str, ...] = (

ws/templates/base.html

@@ -78,7 +78,7 @@
                             <a href="#" class="dropdown-toggle" data-toggle="dropdown" role="button" aria-haspopup="true" aria-expanded="false">Trips <span class="caret"></span></a>
                             <ul class="dropdown-menu">
                                 <li><a href="{% url 'create_trip' %}">Create trip</a></li>
-                                <li><a href="{% url 'upcoming_trips' %}">Upcoming trips</a></li>
+                                <li><a href="{% url 'trips' %}">Upcoming trips</a></li>
                                 <li><a href="{% url 'search_trips' %}">Search trips</a></li>
                             </ul>
                         </li>
@@ -91,7 +91,7 @@
                         </li>
                     {% else %}
                         {# No need for a menu at all -- just show "trips" #}
-                        <li><a href="{% url 'upcoming_trips' %}">Trips</a></li>
+                        <li><a href="{% url 'trips' %}">Trips</a></li>
                     {% endif %}
 
                     {% if user.is_superuser %}

ws/templates/help/leaders/trip_admin.html

@@ -22,9 +22,8 @@ <h4>What is the trip admin view?</h4>
   <p>
     To explore the trip admin interface, first navigate to your trip
     (e.g. through
-    <a href="{% url 'upcoming_trips' %}">upcoming trips</a> or
-    <a href="{% url 'profile' %}">trips you're leading</a>
-    ).
+    <a href="{% url 'trips' %}">upcoming trips</a> or
+    <a href="{% url 'profile' %}">trips you're leading</a>).
     Then, click the "Admin" tab to the right of the selected "Trip" tab.
   </p>
 </div>

ws/templates/home.html

@@ -30,7 +30,12 @@ <h3>
   <h3>Recent trips</h3>
   {% trip_list_table recent_trips %}
   <hr>
-  <p><a href="{% url 'all_trips' %}"><i class="fas fa-history"></i> View all past trips</a></p>
+  <p>
+    <a href="{% url 'trips' %}?after={{ previous_lookup_date|date:"Y-m-d" }}">
+      <i class="fas fa-history"></i>
+      Previous trips
+    </a>
+  </p>
 {% endif %}
 
 

ws/templates/preferences/lottery/edit.html

@@ -129,7 +129,7 @@ <h3 id="ranked_trips">
         {% if not ranked_signups %}
           <p>You're not signed up for any upcoming trips.</p>
           <ol>
-            <li>Begin by signing up for <a href="{% url 'upcoming_trips' %}">any trips you're interested in</a>.</li>
+            <li>Begin by signing up for <a href="{% url 'trips' %}">any trips you're interested in</a>.</li>
             {% if has_paired_par %}<li>Have your partner sign up for the same trips!</li>{% endif %}
             <li>Come back here to rank trips in order of preference.</li>
             <li>The next <a href="{% url 'help-lottery' %}">lottery run</a> will try to place you on your most-preferred trip.</li>
@@ -139,7 +139,7 @@ <h3 id="ranked_trips">
             <p><strong>We strongly recommend signing up for at least four trips!</strong></p>
             <p>
               Without some backup options, it's likely that you won't be placed on a trip at all.
-              <a href="{% url 'upcoming_trips' %}">Sign up for more trips</a> to increase your odds of being placed on a trip.
+              <a href="{% url 'trips' %}">Sign up for more trips</a> to increase your odds of being placed on a trip.
             </p>
           </div>
         {% endif %}

ws/templates/profile/membership.html

@@ -17,7 +17,7 @@ <h1>Pay MITOC membership dues</h1>
     {% endif %}
     enables you to
     <a href="https://mitoc.mit.edu/rentals">rent gear</a> from the office,
-    participate in <a href="{% url 'upcoming_trips' %}">upcoming trips</a>,
+    participate in <a href="{% url 'trips' %}">upcoming trips</a>,
     and stay at <a href="https://mitoc.mit.edu/rentals/cabins">MITOC's cabins</a>.
   </p>
 

ws/templates/trips/__create_or_edit.html

@@ -137,7 +137,7 @@ <h2>Signup</h2>
     {% if form.instance.pk %}
       href="{% url 'view_trip' form.instance.pk %}"
     {% else %}
-      href="{% url 'upcoming_trips' %}"
+      href="{% url 'trips' %}"
     {% endif %}
       >Cancel</a>
 </form>

ws/templates/trips/all/view.html

@@ -56,11 +56,11 @@ <h3>Past trips</h3>
 <hr>
 <p>
   {% if one_year_prior %}
-    {# We hide this link when we're already viewing all past trips #}
-    <a href="{% url 'upcoming_trips' %}?after={{ one_year_prior|date:"Y-m-d" }}">
+    {# We hide this link when there are no trips earlier #}
+    <a href="{% url 'trips' %}?after={{ one_year_prior|date:"Y-m-d" }}">
     <i class="fas fa-history"></i>
     Previous trips
-  </a>
+    </a>
   {% endif %}
   <a class="pull-right" href="https://mitoc.mit.edu"><i class="fas fa-external-link-alt"></i>&nbsp;MITOC home page</a>
 </p>

ws/templates/trips/signup.html

@@ -17,7 +17,7 @@ <h3>Signup for <a href="{% url 'view_trip' trip.pk %}">{{ trip }}</a></h3>
       <button type="submit" class="btn btn-primary">Sign Up</button>
     </form>
   {% else %}
-    <p>Not signing up for any given trip. <a href="{% url 'upcoming_trips' %}">Browse available trips?</a></p>
+    <p>Not signing up for any given trip. <a href="{% url 'trips' %}">Browse available trips?</a></p>
   {% endif %}
 {% endwith %}
 

ws/tests/test_auth.py

@@ -78,7 +78,7 @@ def test_open_pages(self):
             "help-personal_info",
             "help-lottery",
             "help-signups",
-            "upcoming_trips",
+            "trips",
             "stats",
         ]:
             response = self.client.get(reverse(open_url))

ws/tests/views/test_trips.py

@@ -65,7 +65,7 @@ def _expect_link_for_date(soup, datestring):
 
 
 @freeze_time("2019-02-15 12:25:00 EST")
-class UpcomingTripsViewTest(TestCase, Helpers):
+class TripListViewTest(TestCase, Helpers):
     def test_upcoming_trips_without_filter(self):
         """With no default filter, we only show upcoming trips."""
         response, soup = self._get("/trips/")

ws/urls.py

@@ -191,11 +191,11 @@
     path("trips.rss", feeds.UpcomingTripsFeed(), name="rss-upcoming_trips"),
     # By default, `/trips/` shows only upcoming trips
     # Both views support filtering for trips after a certain date, though
-    path("trips/", views.UpcomingTripsView.as_view(), name="upcoming_trips"),
+    path("trips/", views.TripListView.as_view(), name="trips"),
     # With thousands of trips, we can no longer render them all on one page.
     path(
         "trips/all/",
-        RedirectView.as_view(url=reverse_lazy("upcoming_trips"), permanent=True),
+        RedirectView.as_view(url=reverse_lazy("trips"), permanent=True),
         name="all_trips",
     ),
     path("trips/signup/", views.SignUpView.as_view(), name="trip_signup"),

ws/views/participant.py

@@ -537,15 +537,18 @@ def get_context_data(self, **kwargs):
     @staticmethod
     def render_landing_page(request: HttpRequest) -> HttpResponse:
         today = date_utils.local_date()
-        context = {
-            "current_trips": annotated_for_trip_list(
-                models.Trip.objects.filter(trip_date__gte=today).order_by(
-                    "trip_date", "-time_created"
-                )
+        current_trips = annotated_for_trip_list(
+            models.Trip.objects.filter(trip_date__gte=today).order_by(
+                "trip_date", "-time_created"
             )
+        )
+
+        context = {
+            "current_trips": current_trips,
+            "previous_lookup_date": today - models.Trip.TRIPS_LOOKBACK,
         }
 
-        num_trips = len(context["current_trips"])  # Use len to avoid extra query
+        num_trips = len(current_trips)  # Use len to avoid extra query
 
         # If we don't have many upcoming trips, show some recent ones
         if num_trips < 8:

ws/views/signup.py

@@ -139,7 +139,7 @@ def dispatch(self, request, *args, **kwargs):
 
 class DeleteSignupView(DeleteView):
     model = models.SignUp
-    success_url = reverse_lazy("upcoming_trips")
+    success_url = reverse_lazy("trips")
 
     def get(self, request, *args, **kwargs):
         """Request is valid, but method is not (use POST)."""

ws/views/trips.py

@@ -5,7 +5,7 @@
 """
 
 from collections import defaultdict
-from datetime import date, timedelta
+from datetime import date
 from typing import TYPE_CHECKING, Any, cast
 from urllib.parse import urlencode
 
@@ -53,6 +53,8 @@
 # There will never be any *older* trips, so we can save db queries.
 FIRST_TRIP_DATE = date(2015, 1, 10)
 
+TRIPS_LOOKBACK = models.Trip.TRIPS_LOOKBACK
+
 
 class TripView(DetailView):
     """Display the trip to both unregistered users and known participants.
@@ -337,7 +339,7 @@ def dispatch(self, request, *args, **kwargs):
 class DeleteTripView(DeleteView, TripLeadersOnlyView):  # type: ignore[misc]
     forbid_modifying_old_trips = True
     model = models.Trip
-    success_url = reverse_lazy("upcoming_trips")
+    success_url = reverse_lazy("trips")
 
     def get(self, request, *args, **kwargs):
         """Request is valid, but method is not (use POST)."""
@@ -486,7 +488,6 @@ class TripListView(ListView):
     model = models.Trip
     template_name = "trips/all/view.html"
     context_object_name = "trip_queryset"
-    include_past_trips = True
 
     def get_queryset(self):
         trips = super().get_queryset()
@@ -536,13 +537,13 @@ def get_context_data(self, **kwargs):
         if oldest_trip_date >= FIRST_TRIP_DATE:
             # Get approximately one year prior for use in paginating back in time.
             # (need not be exact/handle leap years)
-            context["one_year_prior"] = oldest_trip_date - timedelta(days=365)
+            context["one_year_prior"] = oldest_trip_date - TRIPS_LOOKBACK
 
         # By default, just show upcoming trips.
         context["current_trips"] = trips.filter(trip_date__gte=today)
 
         # However, if we've explicitly opted in to showing past trips, include them.
-        if self.include_past_trips or on_or_after_date:
+        if on_or_after_date:
             # Note that we need to sort past trips in descending order (most recent trips first).
             # This is because we have a cutoff in the past, and it would display strangely to sort ascending.
             context["past_trips"] = trips.filter(trip_date__lt=today).order_by(
@@ -554,16 +555,6 @@ def get_context_data(self, **kwargs):
         return context
 
 
-class UpcomingTripsView(TripListView):
-    """By default, view only upcoming (future) trips.
-
-    If given a date, filter to only trips after that date (which may be past dates!)
-    """
-
-    # Default value, but past trips can appear by including a date filter
-    include_past_trips = False
-
-
 class TripSearchView(ListView, FormView):
     form_class = forms.TripSearchForm