diff --git a/frontends/api/src/generated/v0/api.ts b/frontends/api/src/generated/v0/api.ts index f474a96903..6fe241943a 100644 --- a/frontends/api/src/generated/v0/api.ts +++ b/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} [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, offset?: number, - published?: boolean, options: RawAxiosRequestConfig = {}, ): Promise => { 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} [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, 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)) diff --git a/openapi/specs/v0.yaml b/openapi/specs/v0.yaml index 9812c1eaa0..a506e9437c 100644 --- a/openapi/specs/v0.yaml +++ b/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: diff --git a/testimonials/filters.py b/testimonials/filters.py index f1920d5a5d..328e93dde5 100644 --- a/testimonials/filters.py +++ b/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 diff --git a/testimonials/views.py b/testimonials/views.py index 1bbf67c317..138b6b5767 100644 --- a/testimonials/views.py +++ b/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] diff --git a/testimonials/views_test.py b/testimonials/views_test.py index 191559eef7..1d7858448c 100644 --- a/testimonials/views_test.py +++ b/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,16 +31,30 @@ 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] @@ -47,47 +62,51 @@ def test_attestation_filters(user_client, filter_channels, filter_published): 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)