Fixing authentication issue, and fixing some filtering and test issues (

#1039)

frontends/api/src/generated/v0/api.ts

@@ -5431,7 +5431,6 @@ export const TestimonialsApiAxiosParamCreator = function (
      * @param {number} [limit] Number of results to return per page.
      * @param {Array<string>} [offerors] The offerors the attestation is for
      * @param {number} [offset] The initial index from which to return the results.
-     * @param {boolean} [published] Only return published testimonials
      * @param {*} [options] Override http request option.
      * @throws {RequiredError}
      */
@@ -5440,7 +5439,6 @@ export const TestimonialsApiAxiosParamCreator = function (
       limit?: number,
       offerors?: Array<string>,
       offset?: number,
-      published?: boolean,
       options: RawAxiosRequestConfig = {},
     ): Promise<RequestArgs> => {
       const localVarPath = `/api/v0/testimonials/`
@@ -5475,10 +5473,6 @@ export const TestimonialsApiAxiosParamCreator = function (
         localVarQueryParameter["offset"] = offset
       }
 
-      if (published !== undefined) {
-        localVarQueryParameter["published"] = published
-      }
-
       setSearchParams(localVarUrlObj, localVarQueryParameter)
       let headersFromBaseOptions =
         baseOptions && baseOptions.headers ? baseOptions.headers : {}
@@ -5557,7 +5551,6 @@ export const TestimonialsApiFp = function (configuration?: Configuration) {
      * @param {number} [limit] Number of results to return per page.
      * @param {Array<string>} [offerors] The offerors the attestation is for
      * @param {number} [offset] The initial index from which to return the results.
-     * @param {boolean} [published] Only return published testimonials
      * @param {*} [options] Override http request option.
      * @throws {RequiredError}
      */
@@ -5566,7 +5559,6 @@ export const TestimonialsApiFp = function (configuration?: Configuration) {
       limit?: number,
       offerors?: Array<string>,
       offset?: number,
-      published?: boolean,
       options?: RawAxiosRequestConfig,
     ): Promise<
       (
@@ -5580,7 +5572,6 @@ export const TestimonialsApiFp = function (configuration?: Configuration) {
           limit,
           offerors,
           offset,
-          published,
           options,
         )
       const index = configuration?.serverIndex ?? 0
@@ -5651,7 +5642,6 @@ export const TestimonialsApiFactory = function (
           requestParameters.limit,
           requestParameters.offerors,
           requestParameters.offset,
-          requestParameters.published,
           options,
         )
         .then((request) => request(axios, basePath))
@@ -5707,13 +5697,6 @@ export interface TestimonialsApiTestimonialsListRequest {
    * @memberof TestimonialsApiTestimonialsList
    */
   readonly offset?: number
-
-  /**
-   * Only return published testimonials
-   * @type {boolean}
-   * @memberof TestimonialsApiTestimonialsList
-   */
-  readonly published?: boolean
 }
 
 /**
@@ -5755,7 +5738,6 @@ export class TestimonialsApi extends BaseAPI {
         requestParameters.limit,
         requestParameters.offerors,
         requestParameters.offset,
-        requestParameters.published,
         options,
       )
       .then((request) => request(this.axios, this.basePath))

openapi/specs/v0.yaml

@@ -554,11 +554,6 @@ paths:
         description: The initial index from which to return the results.
         schema:
           type: integer
-      - in: query
-        name: published
-        schema:
-          type: boolean
-        description: Only return published testimonials
       tags:
       - testimonials
       responses:

testimonials/filters.py

@@ -1,15 +1,12 @@
 """Filters for testimonials."""
 
-from django.db.models import Q
 from django_filters import (
-    BooleanFilter,
     FilterSet,
     ModelMultipleChoiceFilter,
 )
 
 from channels.models import FieldChannel
 from learning_resources.models import LearningResourceOfferor
-from main.utils import now_in_utc
 
 
 class AttestationFilter(FilterSet):
@@ -23,16 +20,3 @@ class AttestationFilter(FilterSet):
         label="The offerors the attestation is for",
         queryset=LearningResourceOfferor.objects.all(),
     )
-    published = BooleanFilter(
-        label="Only return published testimonials", method="filter_published"
-    )
-
-    def filter_published(self, queryset, _, value):
-        """Filter only published attestations"""
-
-        if value:
-            return queryset.filter(
-                Q(publish_date__isnull=True) | Q(publish_date__lte=now_in_utc())
-            )
-
-        return queryset

testimonials/views.py

@@ -1,17 +1,17 @@
 """Views for testimonials."""
 
-from django.contrib.auth import get_user_model
+from django.db.models import Q
 from drf_spectacular.utils import extend_schema, extend_schema_view
 from rest_framework.viewsets import ReadOnlyModelViewSet
 
 from learning_resources.views import LargePagination
 from main.filters import MultipleOptionsFilterBackend
+from main.permissions import AnonymousAccessReadonlyPermission
+from main.utils import now_in_utc
 from testimonials.filters import AttestationFilter
 from testimonials.models import Attestation
 from testimonials.serializers import AttestationSerializer
 
-User = get_user_model()
-
 
 @extend_schema_view(
     list=extend_schema(
@@ -29,7 +29,10 @@ class AttestationViewSet(ReadOnlyModelViewSet):
     """Viewset for attestations."""
 
     serializer_class = AttestationSerializer
-    queryset = Attestation.objects.all()
+    queryset = Attestation.objects.filter(
+        Q(publish_date__isnull=True) | Q(publish_date__lte=now_in_utc())
+    ).all()
     pagination_class = LargePagination
     filter_backends = [MultipleOptionsFilterBackend]
     filterset_class = AttestationFilter
+    permission_classes = [AnonymousAccessReadonlyPermission]

testimonials/views_test.py

@@ -6,6 +6,7 @@
 from django.urls import reverse
 
 from channels.factories import FieldChannelFactory
+from learning_resources.factories import LearningResourceOfferorFactory
 from main.utils import now_in_utc
 from testimonials.factories import AttestationFactory
 from testimonials.models import Attestation
@@ -14,13 +15,13 @@
 pytestmark = pytest.mark.django_db
 
 
-def test_attestation_list(user_client):
+def test_attestation_list(client):
     """Test that attestations can be listed"""
 
     attestation_batch = sorted(AttestationFactory.create_batch(5), key=lambda a: a.id)
 
     list_url = reverse("testimonials:v0:testimonials_api-list")
-    response = user_client.get(list_url).json()
+    response = client.get(list_url).json()
 
     assert len(attestation_batch) == response["count"]
 
@@ -30,64 +31,82 @@ def test_attestation_list(user_client):
 
 
 @pytest.mark.parametrize("filter_channels", [True, False])
-@pytest.mark.parametrize("filter_published", [True, False])
-def test_attestation_filters(user_client, filter_channels, filter_published):
-    """Test that attestations can be listed with filters"""
+@pytest.mark.parametrize("filter_offerors", [True, False])
+@pytest.mark.parametrize("filter_offeror_and_channel", [True, False])
+def test_attestation_filters(
+    client, filter_channels, filter_offerors, filter_offeror_and_channel
+):
+    """
+    Test that attestations can be listed with filters
+
+    Some explanation for the filtering options:
+    - You can filter by channel and offeror. This is an "and" search.
+    - Setting filter_offeror_and_channel tests searching for a matching set of
+      offeror and channel. A channel is added to the last attestation in the
+      batch, and an offeror is added to the _last two_ attestations. If this
+      flag is set, the search will be for the offeror and channel added to the
+      last attestation; otherwise, it'll be for the channel and the offeror
+      added to the 4th attestation.
+    """
 
     attestation_batch = sorted(AttestationFactory.create_batch(6), key=lambda a: a.id)
     api_params = {}
 
     if filter_channels:
         # Add a known channel to the last one of these.
-        # We'll put this at the end - if channels and published filters are both
+        # We'll put this at the end - if channels and offerors filters are both
         # active, we'll expect to receive nothing.
         channel = FieldChannelFactory.create()
         attestation_with_channel = attestation_batch[5]
         attestation_with_channel.channels.add(channel)
         attestation_with_channel.save()
         api_params["channels"] = [channel.id]
 
-    if filter_published:
-        # Set the first four to valid publish dates and the last 2 to future
-        # dates (thus making them unpublished)
-        attestation_batch[0].publish_date = None
-        attestation_batch[1].publish_date = None
-
-        past_date = now_in_utc() - timedelta(days=15)
-        attestation_batch[2].publish_date = past_date
-        attestation_batch[3].publish_date = past_date
-
-        future_date = now_in_utc() + timedelta(days=(99 * 365))
-        attestation_batch[4].publish_date = future_date
-        attestation_batch[5].publish_date = future_date
-        Attestation.objects.bulk_update(attestation_batch, fields=["publish_date"])
-
-        api_params["published"] = True
-
-    if filter_published or filter_channels:
+    if filter_offerors:
+        # Add a known offeror to two of the testimonials.
+        # We'll add this to the second to last and to the last ones, so we can
+        # make sure filtering by both offeror and channel works.
+        offeror1 = LearningResourceOfferorFactory.create(code="ocw")
+        attestation_with_offeror = attestation_batch[4]
+        attestation_with_offeror.offerors.add(offeror1)
+        attestation_with_offeror.save()
+        offeror2 = LearningResourceOfferorFactory.create(code="see")
+        attestation_with_offeror = attestation_batch[5]
+        attestation_with_offeror.offerors.add(offeror2)
+        attestation_with_offeror.save()
+
+    if filter_channels or filter_offerors:
         [
             attestation_batch[idx].refresh_from_db()
             for idx, _ in enumerate(attestation_batch)
         ]
 
+    if filter_offerors:
+        if filter_offeror_and_channel:
+            # Test filtering by an offeror and a channel. If we're not filtering
+            # offeror anyway, then this test doesn't make sense.
+            api_params["offerors"] = [offeror2.code]
+        else:
+            api_params["offerors"] = [offeror1.code]
+
     list_url = reverse("testimonials:v0:testimonials_api-list")
-    response = user_client.get(list_url, api_params).json()
+    response = client.get(list_url, api_params).json()
 
-    if not filter_channels and not filter_published:
+    if not filter_channels and not filter_offerors:
         assert len(attestation_batch) == response["count"]
 
-    if filter_channels and filter_published:
-        assert response["count"] == 0
-
-    if filter_channels and not filter_published:
+    if filter_offerors != filter_channels:
         assert response["count"] == 1
 
-    if filter_published and not filter_channels:
-        assert response["count"] == 4
+    if filter_channels and filter_offerors:
+        if filter_offeror_and_channel:
+            assert response["count"] == 1
+        else:
+            assert response["count"] == 0
 
 
-def test_attestation_published(user_client):
-    """Test that just published attestations can be listed"""
+def test_attestation_published(client):
+    """Test that just published attestations are listed"""
 
     attestation_batch = sorted(AttestationFactory.create_batch(4), key=lambda a: a.id)
 
@@ -109,7 +128,7 @@ def test_attestation_published(user_client):
     Attestation.objects.bulk_update(hidden_attestation_batch, fields=["publish_date"])
 
     list_url = reverse("testimonials:v0:testimonials_api-list")
-    response = user_client.get(list_url, {"published": True}).json()
+    response = client.get(list_url).json()
 
     assert response["count"] == len(attestation_batch)