Skip to content

feat(media): add media create-aspect command#144

Merged
binoy14 merged 3 commits intomainfrom
sdk-602
Sep 30, 2025
Merged

feat(media): add media create-aspect command#144
binoy14 merged 3 commits intomainfrom
sdk-602

Conversation

@binoy14
Copy link
Contributor

@binoy14 binoy14 commented Sep 23, 2025

Description

Migrates media create-aspect command to new CLI

What to review

Changes makes sense

Testing

Added

@linear
Copy link

linear bot commented Sep 23, 2025

SDK-602 Migrate `sanity media create-aspect` command to new CLI

usage: npx sanity media create-aspect 

   Create a new aspect definition file.

Examples
  # Create a new aspect definition file.
  sanity media create-aspect

@claude
Copy link

claude bot commented Sep 23, 2025

Claude finished @binoy14's task —— View job


Code Review

Reviewing the PR for issues that need fixing:

Todo List

  • Read and analyze main command file
  • Review test file for coverage and quality
  • Check utility functions and error handling
  • Verify dependencies and imports
  • Identify actionable issues

Issues Found

Issue 1: Template injection vulnerability

  • Lines 57-60 in create-aspect.ts: User input is directly interpolated into TypeScript template without validation
  • The name and title values could contain characters that break the generated code (quotes, backticks, etc.)
  • Fix: Add input sanitization before template generation

Issue 2: Test/implementation mocking inconsistency

  • Implementation uses fileExists() utility (line 50) but tests mock access() directly (line 34)
  • Since fileExists wraps access, tests should mock fileExists instead
  • Fix: Change test to mock fileExists from @sanity/cli-core rather than access

@github-actions
Copy link

github-actions bot commented Sep 23, 2025

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 79.87% 3167 / 3965
🔵 Statements 79.56% 3235 / 4066
🔵 Functions 77.61% 437 / 563
🔵 Branches 70.82% 1425 / 2012
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
packages/@sanity/cli-core/src/config/cli/schemas.ts 100% 100% 100% 100%
packages/@sanity/cli/src/actions/media/getMediaLibraryConfig.ts 100% 100% 100% 100%
packages/@sanity/cli/src/commands/media/create-aspect.ts 100% 83.33% 100% 100%
packages/@sanity/cli/src/util/errorMessages.ts 100% 100% 100% 100%
Generated in workflow #761 for commit 5eedbd4 by the Vitest Coverage Report Action

@binoy14 binoy14 marked this pull request as ready for review September 23, 2025 21:29
@binoy14 binoy14 requested a review from a team as a code owner September 23, 2025 21:29
@binoy14 binoy14 requested review from juice49 and rexxars and removed request for a team September 23, 2025 21:29
rexxars
rexxars previously approved these changes Sep 29, 2025
@binoy14 binoy14 merged commit ea8224f into main Sep 30, 2025
20 checks passed
@binoy14 binoy14 deleted the sdk-602 branch September 30, 2025 22:45
@squiggler-legacy squiggler-legacy bot mentioned this pull request Sep 30, 2025
This was referenced Dec 19, 2025
This was referenced Mar 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants