refactor(project): replace --org flag with org/project positional#223
refactor(project): replace --org flag with org/project positional#223
Conversation
Semver Impact of This PR🟢 Patch (bug fixes) 📋 Changelog PreviewThis is how your changes will appear in the changelog. New Features ✨Cli
Other
Bug Fixes 🐛Upgrade
Other
Documentation 📚
Internal Changes 🔧
🤖 This preview updates automatically when you update the PR. |
|
Codecov Results 📊✅ Patch coverage is 81.08%. Project has 3871 uncovered lines. Files with missing lines (67)
Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
+ Coverage 68.84% 69.85% +1.01%
==========================================
Files 105 105 —
Lines 12862 12841 -21
Branches 0 0 —
==========================================
+ Hits 8854 8970 +116
- Misses 4008 3871 -137
- Partials 0 0 —Generated by Codecov Action |
0ab2bf1 to
421ffa9
Compare
project view now accepts <org>/<project> as a single positional argument instead of separate --org flag and <project> positional, matching the pattern used by event view and issue view. Auto-detect via DSN still works when no argument is provided, including monorepo multi-target.
E2E tests now pass org/project as a single arg instead of --org flag. Renamed test cases to reflect positional args instead of flags.
Deduplicate resolveFromProjectSearch() which was identically implemented in event/view, trace/view, log/view, and project/view. The shared resolveProjectBySlug() in resolve-target.ts handles project slug lookup with consistent error messages and disambiguation prompts.
421ffa9 to
9da8df4
Compare
| case ProjectSpecificationType.Explicit: | ||
| // Direct org/project - single target, no multi-target resolution | ||
| resolvedTargets = [ | ||
| { | ||
| org: parsed.org, | ||
| project: parsed.project, | ||
| orgDisplay: parsed.org, | ||
| projectDisplay: parsed.project, | ||
| }, | ||
| ]; | ||
| break; |
There was a problem hiding this comment.
Bug: The fetchProjectDetails function in project/view.ts incorrectly swallows ApiErrors (like 404s), returning null instead of propagating the error when a non-existent project is specified.
Severity: MEDIUM
Suggested Fix
Modify the catch block in the fetchProjectDetails function within project/view.ts. Instead of catching all errors and returning null, allow ApiError to be re-thrown, similar to how AuthError is handled. This will ensure that API errors like 404s are propagated up and displayed to the user, providing more specific feedback.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: src/commands/project/view.ts#L249-L259
Potential issue: In `project/view.ts`, the `fetchProjectDetails` function catches any
error except `AuthError` and returns `null`. When a user provides an explicit but
non-existent organization or project, the `getProject` call throws an `ApiError` (e.g.,
404 Not Found). This error is caught and converted to `null`, causing the command to
later fail with a generic `buildContextError()` because no projects were found. This
provides an unhelpful error message to the user instead of the specific API error, and
contradicts the command's E2E test expectations which check for "not found" or "404" in
the output.
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
intentional. this is a UX followup.
Summary
project viewwas the last command still using a--orgflag. This replaces it with the<org>/<project>positional arg pattern thatevent viewandissue viewalready use.Changes
The command now accepts a single optional positional:
For explicit and project-search cases, resolution is direct (single target). Auto-detect still goes through
resolveAllTargets()so monorepo multi-project support is preserved.Also cleaned up leftover
--org/--projectreferences in JSDoc comments acrossresolve-target.tsandresolve-issue.ts.Test Plan
bun run typecheckpassesbun test test/e2e/project.test.ts— all 18 tests passbun test test/isolated/resolve-target.test.ts— all 27 tests passbun test test/lib/errors.test.ts— all tests pass