Add frontend media documentation #4313

zackkrida · 2024-05-10T22:00:43Z

Fixes

Description

Adds new comments to media properties in documentation/meta/media_properties/frontend.md, and publishes the generated documentation changes.

I tried to provide information relevant to the context of using these properties in the frontend, rather than purely generic information. I am very open to suggestions on what is useful and consider this a PR that will generate a lot of positive feedback as a "conversation starter" rather than a perfect implementation.

Testing Instructions

Review the produced docs.

Checklist

My pull request has a descriptive title (not a vague title likeUpdate 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.
I ran the DAG documentation generator (just catalog/generate-docs for catalog
PRs) or the media properties generator (just catalog/generate-docs media-props
for the catalog or just api/generate-docs for the API) where applicable.

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 · 2024-05-14T17:46:00Z

frontend/src/types/media.ts

  audio_set?: AudioSet
+  /** A raw list of strings representing musical genres. */
  genres?: string[]
  length?: string


I do not actually know what this length is ☺️

This should be removed.
length is a query parameter, not a property of an AudioDetail.

openverse/frontend/src/utils/search-query-transform.ts

Line 38 in 292946e

lengths: "length",

github-actions · 2024-05-14T17:57:40Z

Full-stack documentation: https://docs.openverse.org/_preview/4313

Please note that GitHub pages takes a little time to deploy newly pushed code, if the links above don't work or you see old versions, wait 5 minutes and try again.

You can check the GitHub pages deployment action list to see the current status of the deployments.

Changed files 🔄:

https://docs.openverse.org/_preview/4313/meta/media_properties/frontend.html

…4025

AetherUnbound · 2024-05-15T17:51:09Z

Removing myself and Staci from reviewing this as we don't have enough subject-matter expertise or context for it.

obulat

Good start for this implementation. I added several suggestions, but don't wan't to block.

I feel like it's wasteful to re-write the comments for each layer of the stack instead of re-using the docs from catalog/API. I don't know what the best implementation of this would be.

frontend/src/types/media.ts

obulat · 2024-05-15T17:41:35Z

frontend/src/types/media.ts

+   *
+   * Corresponds to the `source_name` field of provider results
+   * from the API's `/stats/` endpoints.
+   */


frontend/src/types/media.ts

obulat · 2024-05-15T17:47:58Z

frontend/src/types/media.ts

  audio_set?: AudioSet
+  /** A raw list of strings representing musical genres. */
  genres?: string[]
  length?: string


This should be removed.
length is a query parameter, not a property of an AudioDetail.

openverse/frontend/src/utils/search-query-transform.ts

Line 38 in 292946e

lengths: "length",

frontend/src/types/media.ts

zackkrida · 2024-05-15T18:01:06Z

Thanks @obulat! I agree with your suggestions and the redundancy of this approach. I'll apply your suggestions. Once we get this merged (which will be an improvement over the current lack of docs) we can consider ways of reducing redundancy like having root TypeScript types that are transpiled to Python or something

obulat · 2024-05-15T18:07:03Z

(which will be an improvement over the current lack of docs)

Agree, that's why I approved here :)

frontend/src/types/media.ts

Co-authored-by: Olga Bulat <obulat@gmail.com>

zackkrida · 2024-05-21T13:57:53Z

@dhruvkb could I get a review here this week?

dhruvkb

LGTM! I have a small nit about punctuation but that is leaning a lot towards personal preference so I'll not block.

dhruvkb · 2024-05-21T17:04:40Z

frontend/src/types/media.ts

-  /** the raw name of the creative work, as returned by the API */
+
+  /** The raw name of the creative work, as returned by the API. */


Minor nit/personal preference: This isn't a sentence so it shouldn't start with an uppercase letter and should not end with a fullstop. I'm basing this on how dictionaries present definitions.

Add media documentation

59861b9

openverse-bot added the 🚦 status: awaiting triage Has not been triaged & therefore, not ready for work label May 10, 2024

github-actions bot added the 🧱 stack: frontend Related to the Nuxt frontend label May 10, 2024

zackkrida and others added 2 commits May 14, 2024 13:41

apply diff

d5c1786

Merge branch 'main' into fix-#4025

33b38cf

openverse-bot added 🟨 priority: medium Not blocking but should be addressed soon ✨ goal: improvement Improvement to an existing user-facing feature 📄 aspect: text Concerns the textual material in the repository labels May 14, 2024

zackkrida commented May 14, 2024

View reviewed changes

zackkrida added 2 commits May 14, 2024 14:07

final updates

fa41f05

Merge remote-tracking branch 'refs/remotes/origin/fix-#4025' into fix-#…

f7cb5ea

…4025

zackkrida marked this pull request as ready for review May 14, 2024 18:12

zackkrida requested review from a team as code owners May 14, 2024 18:12

zackkrida requested review from obulat, stacimc and AetherUnbound May 14, 2024 18:12

obulat removed the 🚦 status: awaiting triage Has not been triaged & therefore, not ready for work label May 15, 2024

AetherUnbound requested review from dhruvkb and removed request for AetherUnbound and stacimc May 15, 2024 17:50

obulat approved these changes May 15, 2024

View reviewed changes

zackkrida marked this pull request as draft May 15, 2024 18:25

zackkrida commented May 15, 2024

View reviewed changes

frontend/src/types/media.ts Outdated Show resolved Hide resolved

zackkrida and others added 2 commits May 15, 2024 14:48

Apply suggestions from code review

d95f49d

Co-authored-by: Olga Bulat <obulat@gmail.com>

Light cleanup

0a4d812

zackkrida force-pushed the fix-#4025 branch from e990f7e to 0a4d812 Compare May 15, 2024 18:59

zackkrida marked this pull request as ready for review May 15, 2024 18:59

Merge branch 'main' into fix-#4025

fa2fb4a

dhruvkb approved these changes May 21, 2024

View reviewed changes

zackkrida merged commit 9b27da8 into main May 22, 2024
44 checks passed

zackkrida deleted the fix-#4025 branch May 22, 2024 20:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add frontend media documentation #4313

Add frontend media documentation #4313

zackkrida commented May 10, 2024 •

edited

zackkrida May 14, 2024 •

edited

obulat May 15, 2024

github-actions bot commented May 14, 2024

AetherUnbound commented May 15, 2024

obulat left a comment

obulat May 15, 2024

obulat May 15, 2024

zackkrida commented May 15, 2024

obulat commented May 15, 2024

zackkrida commented May 21, 2024

dhruvkb left a comment

dhruvkb May 21, 2024

		/** the raw name of the creative work, as returned by the API */

		/** The raw name of the creative work, as returned by the API. */

Add frontend media documentation #4313

Add frontend media documentation #4313

Conversation

zackkrida commented May 10, 2024 • edited

Fixes

Description

Testing Instructions

Checklist

Developer Certificate of Origin

zackkrida May 14, 2024 • edited

Choose a reason for hiding this comment

obulat May 15, 2024

Choose a reason for hiding this comment

github-actions bot commented May 14, 2024

AetherUnbound commented May 15, 2024

obulat left a comment

Choose a reason for hiding this comment

obulat May 15, 2024

Choose a reason for hiding this comment

obulat May 15, 2024

Choose a reason for hiding this comment

zackkrida commented May 15, 2024

obulat commented May 15, 2024

zackkrida commented May 21, 2024

dhruvkb left a comment

Choose a reason for hiding this comment

dhruvkb May 21, 2024

Choose a reason for hiding this comment

zackkrida commented May 10, 2024 •

edited

zackkrida May 14, 2024 •

edited