Media reporting e2e tests, single audio support, style updates, and accessibility improvements #1276

zackkrida · 2022-04-14T20:11:29Z

Description

This PR makes a number of improvements to the reporting media code:

Updates the styles of the reporting popup trigger button to match the mockups
Adds the reporting functionality to single audio results
Updates the report form to use an actual submit button and form handler, rather than a click event on a type="button" button.
Updates the reporting API service to require a media type
Passes the media.frontendMediaType property to the reporting service to make the form work with all media tyes
Adds e2e tests for each report type for each media type

I would love some heavy attention applied to the e2e tests. These tests run each type of report (dmca, mature, and other) on the first search result each media-specific search endpoint (search/images, for example). This tests the entire flow, like this:

Choose the desired media type from the content switcher
Search for 'cat'
Click on the first result.
Open the report modal.
Submit the current report (there's one test per media type per report)

Currently the tests mock the various network requests made by the reporting form: the google docs form and the api reporting endpoint. There are also likely utilities in this test that could be used elsewhere, if you have any suggestions for those.

Testing Instructions

Report some images and audio locally
Run the e2e tests locally
View the reporting button styles in

Checklist

My pull request has a descriptive title (not a vague title like Update index.md).
My pull request targets the default branch of the repository (main) or a parent feature branch.
My commit messages follow best practices.
My code follows the established code style of the repository.
I added or updated tests for the changes I made (if applicable).
I added or updated documentation (if applicable).
I tried running the project locally and verified that there are no visible errors.

Developer Certificate of Origin

Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.


Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.

zackkrida · 2022-04-14T20:14:10Z

Side note: the VS Code Playwright extension is amazing! fun example, it understands and lets me run these 'dynamic' tests, it's magical:

zackkrida · 2022-04-14T20:15:38Z

src/components/VAudioDetails/VAudioDetails.vue

+    <header class="flex flex-row justify-between items-center mb-6">
+      <h4 class="text-base lg:text-3xl">
+        {{ $t('audio-details.information') }}
+      </h4>
+      <VContentReportPopover :media="audio" />
+    </header>


This adds the report form and button to the single audio results.

zackkrida · 2022-04-14T20:16:38Z

src/components/VContentReport/VContentReportButton.vue

+    class="report-button font-semibold text-dark-charcoal-70"
  >
-    <span class="md:hidden">{{
+    <span class="text-sr md:text-base md:hidden">{{
      $t('media-details.content-report.short')
    }}</span>
-    <span class="hidden md:inline">{{
+    <span class="text-sr md:text-base hidden md:inline">{{


Style updates to the reporting button so it matches the mockups:

zackkrida · 2022-04-14T20:17:33Z

src/components/VContentReport/VContentReportForm.vue

            :disabled="isSubmitDisabled"
            :focusable-when-disabled="true"
            variant="secondary"
-            @click="handleSubmit"
+            :value="$t('media-details.content-report.form.submit')"


Fun fact: Playwright's selectors for text (page.locate('text="Submit"'), for example) use the value attribute when applied to submit buttons, not the actual button text contents.

TIL! I guess we should add value attribute to all the submit buttons? I wonder if it uses value for all buttons...

zackkrida · 2022-04-14T20:18:14Z

src/components/VContentReport/VContentReportForm.vue

@@ -166,11 +167,13 @@ export default defineComponent({
    const isSubmitDisabled = computed(
      () => selectedReason.value === OTHER && description.value.length < 20
    )
-    const handleSubmit = async () => {
+    const handleSubmit = async (event) => {
+      event.preventDefault()


event.preventDefault()

Prevents redirection to the api POST request endpoint.

test/playwright/e2e/report-media.spec.ts

sarayourfriend · 2022-04-15T05:10:58Z

I imagine I should also mock the search responses?

What do you mean by this? As long as there are tapes for the search requests then they are mocked.

sarayourfriend

Just reviewing the code so far. E2e tests look good to me!

test/playwright/e2e/report-media.spec.ts

sarayourfriend

LGTM!

Co-authored-by: sarayourfriend <24264157+sarayourfriend@users.noreply.github.com>

zackkrida · 2022-04-15T11:35:26Z

I imagine I should also mock the search responses?

@sarayourfriend I wrote this before I fully understood the tapes 😅 disregard!

obulat

Great improvements in the way the report sets the mediaType, and overall report style and flow.
I wonder why I can't see the CI run information?

PS: I merged the main into this branch, and CI is all green!

obulat · 2022-04-16T12:15:41Z

src/components/VContentReport/VContentReportForm.vue

      if (selectedReason.value === DMCA) return
      // Submit report
      try {
        await service.sendReport({
+          mediaType: props.media.frontendMediaType,


I keep forgetting that we can use the frontendMediaType property!

# Conflicts: # src/locales/po-files/openverse.pot

zackkrida added 4 commits April 14, 2022 11:30

init

da2b88b

Use proper form event handling for the content report form

a12749c

Add e2e tests for content reporting

980de76

Fix reporting unit tests

7ff58a9

zackkrida added 🟧 priority: high Stalls work on the project or its dependents ✨ goal: improvement Improvement to an existing user-facing feature 💻 aspect: code Concerns the software code in the repository labels Apr 14, 2022

zackkrida requested a review from a team as a code owner April 14, 2022 20:11

zackkrida requested review from krysal and obulat April 14, 2022 20:11

openverse-bot added this to Needs review in Openverse PRs Apr 14, 2022

Fix play/pause conflict

2f2ff15

zackkrida commented Apr 14, 2022

View reviewed changes

test/playwright/e2e/report-media.spec.ts Outdated Show resolved Hide resolved

zackkrida added 5 commits April 14, 2022 16:26

Fix reporting endpoint regex

b38e3cf

Docs cleanup

03831be

Update POT file

0308ce0

Fix test assertions to be simpler and not depend on API_URL

455531e

Small comment removal

3eb1d59

sarayourfriend reviewed Apr 15, 2022

View reviewed changes

test/playwright/e2e/report-media.spec.ts Outdated Show resolved Hide resolved

sarayourfriend approved these changes Apr 15, 2022

View reviewed changes

Update test/playwright/e2e/report-media.spec.ts

a2c6839

Co-authored-by: sarayourfriend <24264157+sarayourfriend@users.noreply.github.com>

zackkrida added this to the Full E2E Test Suite milestone Apr 15, 2022

obulat approved these changes Apr 16, 2022

View reviewed changes

Openverse PRs automation moved this from Needs review to Reviewer approved Apr 16, 2022

Merge branch 'main' into audio-reporting

2de0069

# Conflicts: # src/locales/po-files/openverse.pot

zackkrida merged commit d3f749b into main Apr 16, 2022

Openverse PRs automation moved this from Reviewer approved to Merged! Apr 16, 2022

zackkrida deleted the audio-reporting branch April 16, 2022 16:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Media reporting e2e tests, single audio support, style updates, and accessibility improvements #1276

Media reporting e2e tests, single audio support, style updates, and accessibility improvements #1276

zackkrida commented Apr 14, 2022 •

edited

zackkrida commented Apr 14, 2022

zackkrida Apr 14, 2022

zackkrida Apr 14, 2022

zackkrida Apr 14, 2022 •

edited

obulat Apr 15, 2022

zackkrida Apr 14, 2022 •

edited

sarayourfriend commented Apr 15, 2022

sarayourfriend left a comment

sarayourfriend left a comment

zackkrida commented Apr 15, 2022

obulat left a comment •

edited

obulat Apr 16, 2022

Media reporting e2e tests, single audio support, style updates, and accessibility improvements #1276

Media reporting e2e tests, single audio support, style updates, and accessibility improvements #1276

Conversation

zackkrida commented Apr 14, 2022 • edited

Description

Testing Instructions

Checklist

Developer Certificate of Origin

zackkrida commented Apr 14, 2022

zackkrida Apr 14, 2022

Choose a reason for hiding this comment

zackkrida Apr 14, 2022

Choose a reason for hiding this comment

zackkrida Apr 14, 2022 • edited

Choose a reason for hiding this comment

obulat Apr 15, 2022

Choose a reason for hiding this comment

zackkrida Apr 14, 2022 • edited

Choose a reason for hiding this comment

sarayourfriend commented Apr 15, 2022

sarayourfriend left a comment

Choose a reason for hiding this comment

sarayourfriend left a comment

Choose a reason for hiding this comment

zackkrida commented Apr 15, 2022

obulat left a comment • edited

Choose a reason for hiding this comment

obulat Apr 16, 2022

Choose a reason for hiding this comment

zackkrida commented Apr 14, 2022 •

edited

zackkrida Apr 14, 2022 •

edited

zackkrida Apr 14, 2022 •

edited

obulat left a comment •

edited