Skip to content

Commit

Permalink
mature -> sensitive frontend copy and code (#3006)
Browse files Browse the repository at this point in the history
* `mature` -> `sensitive` frontend copy and code

* Map content report parameters to API expectations

* Update API to handle sensitive reasons for forwards compat

* Update api/api/serializers/media_serializers.py

Co-authored-by: Olga Bulat <[email protected]>

* Undo accidental import change

---------

Co-authored-by: Olga Bulat <[email protected]>
  • Loading branch information
sarayourfriend and obulat authored Sep 12, 2023
1 parent 5b9db79 commit 9952bab
Show file tree
Hide file tree
Showing 24 changed files with 106 additions and 34 deletions.
40 changes: 40 additions & 0 deletions api/api/serializers/media_serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,17 @@ class Meta:
fields = ["identifier", "reason", "description"]
read_only_fields = ["identifier"]

def to_internal_value(self, data):
"""
Map data before validation.
See ``MediaReportRequestSerializer::_map_reason`` docstring for
further explanation.
"""

data["reason"] = self._map_reason(data["reason"])
return super().to_internal_value(data)

def validate(self, attrs):
if (
attrs["reason"] == "other"
Expand All @@ -373,8 +384,37 @@ def validate(self, attrs):
raise serializers.ValidationError(
"Description must be at least be 20 characters long"
)

return attrs

def _map_reason(self, value):
"""
Map `sensitive` to `mature` for forwards compatibility.
This is an interim implementation until the API is updated
to use the new "sensitive" terminology.
Once the API is updated to use "sensitive" as the designator
rather than the current "mature" term, this function should
be updated to reverse the mapping, that is, map `mature` to
`sensitive`, for backwards compatibility.
Note: This cannot be implemented as a simpler `validate_reason` method
on the serializer because field validation runs _before_ validators
declared on the serializer. This means the choice field's validation
will complain about `reason` set to the incorrect value before we have
a chance to map it to the correct value.
This could be mitigated by adding all values, current, future, and
deprecated, to the model field. However, that requires a migration
each time we make that change, and would send an incorrect message
about our data expectations. It's cleaner and more consistent to map
the data up-front, at serialization time, to prevent any confusion at
the data model level.
"""

return "mature" if value == "sensitive" else value


########################
# Response serializers #
Expand Down
6 changes: 6 additions & 0 deletions api/test/unit/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,17 @@
from fakeredis import FakeRedis

from api.serializers.audio_serializers import (
AudioReportRequestSerializer,
AudioSearchRequestSerializer,
AudioSerializer,
)
from api.serializers.image_serializers import (
ImageReportRequestSerializer,
ImageSearchRequestSerializer,
ImageSerializer,
)
from api.serializers.media_serializers import (
MediaReportRequestSerializer,
MediaSearchRequestSerializer,
MediaSerializer,
)
Expand Down Expand Up @@ -66,6 +69,7 @@ class MediaTypeConfig:
mature_factory: MediaFactory
search_request_serializer: MediaSearchRequestSerializer
model_serializer: MediaSerializer
report_serializer: MediaReportRequestSerializer


MEDIA_TYPE_CONFIGS = {
Expand All @@ -78,6 +82,7 @@ class MediaTypeConfig:
mature_factory=model_factories.MatureImageFactory,
search_request_serializer=ImageSearchRequestSerializer,
model_serializer=ImageSerializer,
report_serializer=ImageReportRequestSerializer,
),
"audio": MediaTypeConfig(
media_type="audio",
Expand All @@ -88,6 +93,7 @@ class MediaTypeConfig:
mature_factory=model_factories.MatureAudioFactory,
search_request_serializer=AudioSearchRequestSerializer,
model_serializer=AudioSerializer,
report_serializer=AudioReportRequestSerializer,
),
}

Expand Down
32 changes: 32 additions & 0 deletions api/test/unit/serializers/test_media_serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -248,3 +248,35 @@ def test_index_is_only_set_if_matches_media_type(
)
assert serializer.is_valid() == is_valid
assert serializer.validated_data.get("index") == (index if is_valid else None)


@pytest.mark.django_db
def test_report_serializer_maps_sensitive_reason_to_mature(media_type_config):
media = media_type_config.model_factory.create()
serializer = media_type_config.report_serializer(
data={
"identifier": media.identifier,
"reason": "sensitive",
"description": "Boop beep this is sensitive, whoa!",
}
)

serializer.is_valid(raise_exception=True)

assert serializer.validated_data["reason"] == "mature"


@pytest.mark.django_db
def test_report_serializer_accepts_mature_reason(media_type_config):
media = media_type_config.model_factory.create()
serializer = media_type_config.report_serializer(
data={
"identifier": media.identifier,
"reason": "mature",
"description": "Boop beep this is sensitive, whoa!",
}
)

serializer.is_valid(raise_exception=True)

assert serializer.validated_data["reason"] == "mature"
2 changes: 1 addition & 1 deletion frontend/feat/feature-flags.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"staging": "switchable",
"production": "disabled"
},
"description": "Mark 50% of results as mature to test content safety.",
"description": "Mark 50% of results as sensitive to test content safety.",
"defaultState": "off",
"storage": "cookie"
},
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/constants/content-report.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
export const DMCA = "dmca"
export const MATURE = "mature"
export const SENSITIVE = "sensitive"
export const OTHER = "other"

export const reasons = [DMCA, MATURE, OTHER] as const
export const reasons = [DMCA, SENSITIVE, OTHER] as const

export type ReportReason = (typeof reasons)[number]

Expand Down
8 changes: 2 additions & 6 deletions frontend/src/locales/scripts/en.json5
Original file line number Diff line number Diff line change
Expand Up @@ -333,10 +333,6 @@
medium: "Medium",
large: "Large",
},
mature: {
title: "Search Settings",
enable: "Enable Mature Content",
},
safeBrowsing: {
title: "Safe Browsing",
desc: "Content marked as {sensitive} is not shown by default.",
Expand Down Expand Up @@ -512,8 +508,8 @@
form: "DMCA form",
open: "Open form",
},
mature: {
option: "Contains mature content",
sensitive: {
option: "Contains sensitive content",
subLabel: "Optional",
placeholder: "Optionally, provide a description.",
},
Expand Down
1 change: 0 additions & 1 deletion frontend/src/types/media.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ export interface Media {
tags: Tag[]
fields_matched?: string[]

mature: boolean
sensitivity: Sensitivity[]
isSensitive: boolean
}
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/utils/content-safety.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {
*
* @param id - the ID of the item for which to calculate the flags
* @param frac - the fraction of items to probabilistically flag
* @returns an array of strings representing the mature flags
* @returns an array of strings representing the sensitivity flags
*/
export const getFakeSensitivities = (id: string, frac = 0.5): Sensitivity[] => {
const random = prng(hash(id))()
Expand All @@ -39,6 +39,6 @@ export const getFakeSensitivities = (id: string, frac = 0.5): Sensitivity[] => {
sensitivity.push(TEXT_FILTERED)
}

log("Fake mature", id, sensitivity)
log("Fake sensitive", id, sensitivity)
return sensitivity
}
2 changes: 1 addition & 1 deletion frontend/src/utils/decode-media-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ export const decodeMediaData = <T extends Media>(
media: ApiMedia,
mediaType: T["frontendMediaType"]
): T => {
// Fake ~50% of results as mature.
// Fake ~50% of results as sensitive.
const featureFlagStore = useFeatureFlagStore()
const sensitivity =
featureFlagStore.isOn("fake_sensitive") &&
Expand Down
4 changes: 2 additions & 2 deletions frontend/test/locales/ar.json
Original file line number Diff line number Diff line change
Expand Up @@ -213,8 +213,8 @@
"subLabel": "مطلوب",
"placeholder": "الرجاء إدخال 20 حرفًا على الأقل."
},
"mature": {
"option": "هل يحتوي على محتويات للبالغين",
"sensitive": {
"option": "يحتوي على محتوى حساس",
"subLabel": "اختياري",
"placeholder": "اختياريًا ، قدم وصفًا."
},
Expand Down
5 changes: 3 additions & 2 deletions frontend/test/locales/ru.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,10 @@
"contentReport": {
"short": "Пожаловаться",
"form": {
"mature": {
"sensitive": {
"subLabel": "Необязательно",
"option": "Содержимое для взрослой аудитории"
"option": "Содержит конфиденциальный контент",
"placeholder": "По желанию укажите описание"
},
"other": {
"placeholder": "Введите не менее 20 символов.",
Expand Down
8 changes: 4 additions & 4 deletions frontend/test/playwright/e2e/report-media.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,13 +58,13 @@ const submitDmcaReport = async (page: Page, context: BrowserContext) => {
return expect(newPage.url()).toContain("https://docs.google.com/forms")
}

// todo: Test a mature report with the optional description field
const submitMatureContentReport = async (
// todo: Test a sensitive report with the optional description field
const submitSensitiveContentReport = async (
page: Page,
context: BrowserContext
) => {
await mockReportingEndpoint(context)
await page.click('text="Contains mature content"')
await page.click('text="Contains sensitive content"')
const response = await submitApiReport(page)
return expect(response.status()).toBe(200)
}
Expand All @@ -86,7 +86,7 @@ test.beforeEach(async ({ context }) => {

const reports = {
dmca: submitDmcaReport,
mature: submitMatureContentReport,
sensitive: submitSensitiveContentReport,
other: submitOtherReport,
}

Expand Down
2 changes: 1 addition & 1 deletion frontend/test/playwright/e2e/search-query-client.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ test.describe("search query on CSR", () => {
.getByLabel(/Turn on sensitive content fetching and blurring/i)
.check()
await page
.getByLabel(/Mark 50% of results as mature to test content safety./i)
.getByLabel(/Mark 50% of results as sensitive to test content safety./i)
.check()

await goToSearchTerm(page, "cat", { mode: "CSR" })
Expand Down
2 changes: 1 addition & 1 deletion frontend/test/playwright/e2e/search-query-server.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { ALL_MEDIA, AUDIO, IMAGE } from "~/constants/media"
* 3. query parameters are used to set the filter data:
* 3a. One of each values for `all` content
* 3b. Several query values - several filter checkboxes
* 3c. Mature filter
* 3c. Sensitive results filter
* 3d. Query parameters that are not used for current media type are discarded
* All of these tests test server-generated search page, not the one generated on the client
*/
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 1 addition & 2 deletions frontend/test/unit/fixtures/audio.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ export const getAudioObj = (overrides = {}) =>
alt_files: null,
attribution:
'"La vie des bêtes" by AS-POTIRONT! is licensed under CC BY-NC-SA 2.5. To view a copy of this license, visit https://creativecommons.org/licenses/by-nc-sa/2.5/.',
mature: null,
thumbnail:
"https://localhost:8000/v1/audio/e19345b8-6937-49f7-a0fd-03bf057efc28/thumb",
waveform:
Expand All @@ -48,7 +47,7 @@ export const getAudioObj = (overrides = {}) =>
related_url:
"http://localhost:8000/v1/audio/e19345b8-6937-49f7-a0fd-03bf057efc28/recommendations",
peaks: [],
isSensitive: false,
unstable_sensitivity: [],
},
overrides
)
1 change: 0 additions & 1 deletion frontend/test/unit/fixtures/image.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ export const image = {
attribution:
'"Cat cafe in Seoul" by toel-uru is licensed under CC BY-NC-SA 2.0. To view a copy of this license, visit https://creativecommons.org/licenses/by-nc-sa/2.0/.',
fields_matched: ["description", "tags.name", "title"],
mature: false,
height: 681,
width: 1024,
thumbnail:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ const getDmcaInput = () =>
screen.queryByRole("radio", {
name: /Infringes copyright/i,
})
const getMatureInput = () =>
const getSensitiveInput = () =>
screen.queryByRole("radio", {
name: /mature/i,
name: /sensitive/i,
})
const getOtherInput = () =>
screen.queryByRole("radio", {
Expand Down Expand Up @@ -75,7 +75,7 @@ describe("VContentReportForm", () => {
it("should contain the correct contents", async () => {
await render(VContentReportForm, options)
expect(getDmcaInput()).toBeVisible()
expect(getMatureInput()).toBeVisible()
expect(getSensitiveInput()).toBeVisible()
expect(getOtherInput()).toBeVisible()
expect(getCancelButton()).toBeVisible()
// By default, DMCA is selected, and we show a link to
Expand All @@ -86,7 +86,7 @@ describe("VContentReportForm", () => {

it("should render thank you note when report is sent", async () => {
const { queryByText } = render(VContentReportForm, options)
await fireEvent.click(getMatureInput())
await fireEvent.click(getSensitiveInput())
await fireEvent.click(getReportButton())

// Submission successful message
Expand All @@ -99,7 +99,7 @@ describe("VContentReportForm", () => {
ReportService.sendReport = () => Promise.reject()

const { queryByText } = render(VContentReportForm, options)
await fireEvent.click(getMatureInput())
await fireEvent.click(getSensitiveInput())
await fireEvent.click(getReportButton())

// Submission error message
Expand Down Expand Up @@ -130,16 +130,16 @@ describe("VContentReportForm", () => {
getDescriptionTextarea()
})

it("should dispatch SEND_CONTENT_REPORT on next when mature is selected", async () => {
it("should dispatch SEND_CONTENT_REPORT on next when sensitive is selected", async () => {
ReportService.sendReport = jest.fn()

render(VContentReportForm, options)
await fireEvent.click(getMatureInput())
await fireEvent.click(getSensitiveInput())
await fireEvent.click(getReportButton())

expect(ReportService.sendReport).toHaveBeenCalledWith({
identifier: props.media.id,
reason: "mature",
reason: "sensitive",
mediaType: props.media.frontendMediaType,
description: "",
})
Expand Down

0 comments on commit 9952bab

Please sign in to comment.