Repository navigation
fix: copy DAG and document name instead of absolute file path - #2509
Merged
Merged
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughChangesFile path metadata removal
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ui/src/features/dags/components/dag-details/DAGHeader.tsx`:
- Around line 38-66: Update the copied-state logic in DAGHeader’s copyName flow
to track the copied name rather than a boolean, and retain the timeout handle in
a ref so each new copy clears the previous timeout before scheduling another.
Ensure the copied feedback only remains associated with the name most recently
copied and resets after the timeout.
- Around line 235-242: Update the copy button’s accessibility feedback in the
displayName block and copiedName state so assistive technology announces
successful copying: use a dynamic aria-label reflecting the copied state or add
an aria-live status, while preserving the existing “Copy name” label before
copying.
- Around line 56-64: Update the fallback copy handling in the catch path to
track the boolean result of document.execCommand('copy'), clean up the temporary
textarea in a finally block, and return without calling setCopiedName when
copying fails. Only setCopiedName(true) and schedule its reset after a
successful fallback copy.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1e1df753-0b3d-484e-b846-9bf7f4d5743d
📒 Files selected for processing (16)
api/v1/api.gen.goapi/v1/api.yamlinternal/service/frontend/api/v1/dags.gointernal/service/frontend/api/v1/dags_test.gointernal/service/frontend/api/v1/docs_response.goui/src/api/v1/schema.tsui/src/features/dags/components/dag-details/DAGDetailsContent.tsxui/src/features/dags/components/dag-details/DAGDetailsPanel.tsxui/src/features/dags/components/dag-details/DAGDetailsSidePanel.tsxui/src/features/dags/components/dag-details/DAGHeader.tsxui/src/features/dags/components/dag-details/__tests__/DAGDetailsPanel.test.tsxui/src/features/dags/components/dag-details/__tests__/DAGDetailsSidePanel.test.tsxui/src/pages/dags/dag/__tests__/index.test.tsxui/src/pages/dags/dag/index.tsxui/src/pages/docs/components/DocEditor.tsxui/src/pages/docs/components/__tests__/DocEditor.test.tsx
💤 Files with no reviewable changes (13)
- ui/src/pages/dags/dag/index.tsx
- ui/src/features/dags/components/dag-details/tests/DAGDetailsSidePanel.test.tsx
- ui/src/features/dags/components/dag-details/DAGDetailsPanel.tsx
- internal/service/frontend/api/v1/docs_response.go
- ui/src/features/dags/components/dag-details/DAGDetailsSidePanel.tsx
- ui/src/pages/dags/dag/tests/index.test.tsx
- ui/src/features/dags/components/dag-details/tests/DAGDetailsPanel.test.tsx
- ui/src/features/dags/components/dag-details/DAGDetailsContent.tsx
- ui/src/pages/docs/components/tests/DocEditor.test.tsx
- internal/service/frontend/api/v1/dags.go
- api/v1/api.yaml
- internal/service/frontend/api/v1/dags_test.go
- ui/src/api/v1/schema.ts
# Conflicts: # api/v1/api.gen.go
This was referenced Aug 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The copy button next to the DAG title and the document title copied the absolute file path on disk. It now copies the name shown next to it.
DAGHeader: copies the displayed DAG name (dagRun.name || dag.name), tooltipCopy name: <name>, addsaria-label="Copy name".DocEditor: copies the document name (doc.title, falling back to the doc path basename). The button is no longer gated on the response carrying a file path.API
With the UI no longer consuming it, the absolute-path field is dropped from the HTTP surface:
GET /dags/{fileName}response:filePathremovedDAGFile(DAG list):filePathremovedDocResponse:filePathremovedSyncItem.filePath/SyncItemDiffResponse.filePathare untouched — those are relative paths that identify a sync item, not disk paths.Domain models keep their locations (
dag.Location,docs.Doc.FilePath); only the API stopped exposing absolute paths.Breaking: external REST/MCP clients reading
filePathfrom DAG list, DAG details, or doc responses no longer receive it. Nothing in this repo consumed it.Notes
api/v1/api.gen.gowas regenerated with oapi-codegen v2.5.1 — the version stamped in the checked-in file.make apicurrently fails before generating, on a pre-existing spec validation error also present onmain:Test plan
go build ./...,go vet,gofmt -lcleango test ./internal/service/frontend/api/v1/passespnpm typecheckclean for touched filespnpm vitest runover dag-details, dags/dag, docs: 17 files / 72 tests passSummary by cubic
Fixes the copy buttons next to DAG and document titles to copy the displayed name instead of the absolute disk path, with more robust clipboard handling and accessible feedback. Removes absolute
filePathfrom public API responses and the docs domain model to avoid leaking disk paths.Bug Fixes
DAGHeader: copies the displayed DAG name (dagRun.name || dag.name), tooltip "Copy name: ", addsaria-label="Copy name", scopes the copied indicator to the exact name, usescopyTextwith a secure-context fallback, and cleans up timers.DocEditor: copies the document name (doc.title, falling back to the path basename); button no longer depends on an absolute file path; adds aria-live feedback scoped to the copied name; usescopyTextand cleans up timers.Migration
filePathfromGET /dags/{fileName}response,DAGFilein DAG list, andDocResponse.docs.Doc.FilePathfrom the domain model;dag.Locationremains.dagRun.name || dag.name) ordoc.titlefor copy/actions.SyncItem.filePath(relative) is unchanged.Written for commit 1e707d6. Summary will update on new commits.
Summary by CodeRabbit