fix: metadata language tags media-component#7956
Conversation
|
Warning Rate limit exceeded@tanflem has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 10 minutes and 42 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughReplaces local language-selection logic in media components with centralized helpers: adds getPreferredContent and getPreferredItems utilities and types, uses them to derive title/short/long content and study questions, and computes a layered metadata language tag fallback. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant MC as MediaComponent
participant LU as LanguageUtils
participant Video as VideoData
MC->>Video: read title/snippet/description, studyQuestions, metadataLanguages
MC->>LU: getPreferredContent(title[], metadataLanguages)
LU-->>MC: {value, languageId, bcp47} (titleContent)
MC->>LU: getPreferredContent(description[], metadataLanguages)
LU-->>MC: {value, languageId, bcp47} (longDescriptionContent)
MC->>LU: getPreferredContent(snippet[], metadataLanguages)
LU-->>MC: {value, languageId, bcp47} (shortDescriptionContent)
MC->>LU: getPreferredItems(studyQuestions[], metadataLanguages)
LU-->>MC: string[] (preferred studyQuestions)
MC->>MC: compute metadataLanguageTag: title.bcp47 ?? longDesc.bcp47 ?? shortDesc.bcp47 ?? 'en'
MC->>MC: render using .value fields
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
View your CI Pipeline Execution ↗ for commit bf0b1ca
☁️ Nx Cloud last updated this comment at |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/arclight/src/app/v2/[...route]/_media-components/index.ts(3 hunks)apps/arclight/src/lib/getLanguageIdsFromTags.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/base.mdc)
**/*.{ts,tsx,js,jsx}: Use early returns whenever possible to make the code more readable.
Use descriptive variable and function/const names.
Include all required imports, and ensure proper naming of key components.
Files:
apps/arclight/src/lib/getLanguageIdsFromTags.tsapps/arclight/src/app/v2/[...route]/_media-components/index.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/base.mdc)
Define a type if possible.
Files:
apps/arclight/src/lib/getLanguageIdsFromTags.tsapps/arclight/src/app/v2/[...route]/_media-components/index.ts
apps/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/apps.mdc)
apps/**/*.{js,jsx,ts,tsx}: Always use MUI over HTML elements; avoid using CSS or tags.
Use descriptive variable and function/const names. Also, event functions should be named with a “handle” prefix, like “handleClick” for onClick and “handleKeyDown” for onKeyDown.
Implement accessibility features on elements. For example, a tag should have a tabindex=“0”, aria-label, on:click, and on:keydown, and similar attributes.
Files:
apps/arclight/src/lib/getLanguageIdsFromTags.tsapps/arclight/src/app/v2/[...route]/_media-components/index.ts
🧬 Code graph analysis (2)
apps/arclight/src/lib/getLanguageIdsFromTags.ts (1)
apps/arclight/src/app/v2/[...route]/_metadata-language-tags/languages.ts (1)
languages(7-108)
apps/arclight/src/app/v2/[...route]/_media-components/index.ts (1)
apps/arclight/src/lib/getLanguageIdsFromTags.ts (2)
getPreferredContent(122-157)getPreferredItems(163-183)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: danger (22)
- GitHub Check: build (22)
- GitHub Check: lint (22)
🔇 Additional comments (7)
apps/arclight/src/lib/getLanguageIdsFromTags.ts (3)
108-117: LGTM!The type definitions are clear, well-structured, and appropriately expose the shape of content items and language metadata used by the helper functions.
118-157: LGTM!The function correctly implements a two-tier language selection strategy: prioritizes the primary language, then falls back to other available languages. Edge cases (empty arrays, no matches) are handled appropriately with safe defaults.
159-183: LGTM!The function correctly prioritizes items in the primary language and falls back to items in any available fallback language. Note that the fallback could return items from multiple languages mixed together if the primary language has no items. This design choice provides available content rather than returning nothing, which is appropriate for collections like study questions.
apps/arclight/src/app/v2/[...route]/_media-components/index.ts (4)
7-11: LGTM!The updated imports correctly reference the centralized language-selection helpers, improving code maintainability by eliminating duplication.
363-371: LGTM!The content extraction correctly uses the centralized helper to obtain language-aware content with full metadata (value, languageId, bcp47) for downstream use.
414-420: LGTM!The refactored code correctly extracts values from the content objects and uses
getPreferredItemsfor study questions, maintaining consistent language-aware content selection throughout.
421-425: LGTM!The layered fallback strategy for
metadataLanguageTagis sensible: it prioritizes the title's BCP47 tag, falls back through descriptions, and ultimately defaults to'en'. This provides a robust guarantee that a valid language tag is always present.
|
@coderabbitai generate docstrings |
✅ Actions performedDocstrings generation started. |
…in-media-components-endpoint` Docstrings generation was requested by @tanflem. * #7956 (comment) The following files were modified: * `apps/arclight/src/lib/getLanguageIdsFromTags.ts`
|
Note Generated docstrings for this pull request at #7961 |
…in-media-components-endpoint` (#7961) Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…ags-in-media-components-endpoint
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/arclight/src/lib/getLanguageIdsFromTags.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/base.mdc)
**/*.{ts,tsx,js,jsx}: Use early returns whenever possible to make the code more readable.
Use descriptive variable and function/const names.
Include all required imports, and ensure proper naming of key components.
Files:
apps/arclight/src/lib/getLanguageIdsFromTags.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/base.mdc)
Define a type if possible.
Files:
apps/arclight/src/lib/getLanguageIdsFromTags.ts
apps/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/apps.mdc)
apps/**/*.{js,jsx,ts,tsx}: Always use MUI over HTML elements; avoid using CSS or tags.
Use descriptive variable and function/const names. Also, event functions should be named with a “handle” prefix, like “handleClick” for onClick and “handleKeyDown” for onKeyDown.
Implement accessibility features on elements. For example, a tag should have a tabindex=“0”, aria-label, on:click, and on:keydown, and similar attributes.
Files:
apps/arclight/src/lib/getLanguageIdsFromTags.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: lint (22)
- GitHub Check: test (22, 2/3)
- GitHub Check: build (22)
🔇 Additional comments (4)
apps/arclight/src/lib/getLanguageIdsFromTags.ts (4)
77-82: LGTM!The docstring clearly documents the function's behavior, parameters, and return value.
104-113: The sort comparator fix is correct.The addition of the check at line 108 properly addresses the previous review comment about antisymmetry violations. The comparator now correctly handles the case when both items are not found in
neededBcp47Tags, ensuring deterministic sorting behavior.
115-123: LGTM!The type definitions are well-structured with descriptive field names and appropriate nullability.
191-195: No changes needed
The fallback correctly returns values from all metadata languages per the function’s docstring.
|
I see you added the "on stage" label, I'll get this merged to the stage branch! |
tanflem
deleted the
tannerfleming/eng-3290-fix-metadatalanguagetags-in-media-components-endpoint
branch
2 participants
Summary by CodeRabbit