feat(ui): support direct data-url dataset paths#251
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughAdds ChangesSingle-dataset path routing
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant useDataPoint
participant DataURL
Browser->>useDataPoint: Open /<encoded-id> with --data-url
useDataPoint->>DataURL: Fetch /dataset/<encoded-id>
DataURL-->>useDataPoint: Return dataset detail
useDataPoint-->>Browser: Render the selected dataset
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@cmd/ui.go`:
- Around line 50-51: Update the HTTP(S) route description in cmd/ui.go to use
“/<encoded-id>” instead of “/<id>”, matching the terminology on line 49 and
indicating that dataset IDs with special characters must be URL-encoded.
In `@docs/src/content/docs/commands/ui.mdx`:
- Line 101: Update the documentation sentence describing the final path segment
of <data-url> to call the dataset ID “encoded” rather than “decoded,” consistent
with the <encoded-id> terminology and implementation.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3e495713-2338-44d2-a2b9-6db64b5b2511
📒 Files selected for processing (10)
cmd/ui.godocs/src/content/docs/ci-cd/github-action.mdxdocs/src/content/docs/commands/ui.mdxui/src/composables/useDataPoint.test.tsui/src/composables/useDataPoint.tsui/src/composables/useUrlRouter.test.tsui/src/composables/useUrlRouter.tsui/src/lib/pathRoute.test.tsui/src/lib/pathRoute.tsui/src/views/Dashboard.vue
5fb3778 to
bea807c
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/composables/useUrlRouter.ts`:
- Around line 254-260: Update initFromUrl() to call syncUrlToState() immediately
after registering the watcher, but only when applyParams(params) returns
applied=true. Do not perform this initial synchronization when initialization
fails, so retryable error URLs preserve their original query parameters.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro
Run ID: b3dc55e9-af0f-43fd-bdb0-5e321077a8fc
📒 Files selected for processing (10)
cmd/ui.godocs/src/content/docs/ci-cd/github-action.mdxdocs/src/content/docs/commands/ui.mdxui/src/composables/useDataPoint.test.tsui/src/composables/useDataPoint.tsui/src/composables/useUrlRouter.test.tsui/src/composables/useUrlRouter.tsui/src/lib/pathRoute.test.tsui/src/lib/pathRoute.tsui/src/views/Dashboard.vue
🚧 Files skipped from review as they are similar to previous changes (8)
- ui/src/lib/pathRoute.ts
- ui/src/lib/pathRoute.test.ts
- ui/src/views/Dashboard.vue
- cmd/ui.go
- ui/src/composables/useDataPoint.test.ts
- docs/src/content/docs/ci-cd/github-action.mdx
- ui/src/composables/useUrlRouter.test.ts
- ui/src/composables/useDataPoint.ts
|
hi @fahimfaisaal can I get you review this PR when you have a chance ? Thanks |
|
@hfl0506 I will review, all of your pending PRs by today, In sha Allah |
No worries man, thanks for helping for reviewing! Do you have a discord or something ? |
|
Updated the path behavior so I also added integration-style Vitest coverage for catalog selection, caching, retries, and direct path loading. The docs now use neutral examples and make clear that |
|
@hfl0506 Yeah, I have a Discord personal account username: |
|
@hfl0506 we can connect on discord |
Related Issue
Closes #244
Changes
Add direct dataset deep links via / when --data-url is set.
Fetch only /dataset/ without loading the catalog
first.
Preserve existing root, HTML/static asset, trailing-slash,
embedded-data, and file:// behavior.
Reuse existing query handling for chart, group, legacy, and per-chart
settings.
Ignore and omit ?id=/?d= in path mode while preserving the pathname.
Show full-page load errors with retry for failed direct-detail requests.
Add coverage for path parsing, encoded IDs, one-call loading, retry, URL
apply/sync, and regressions.
Document SPA fallback requirements in CLI help and user documentation.
Test Result
Case 1
Summary by CodeRabbit
/<dataset-id>), with path identity taking precedence overid/d.--data-urlover HTTP(S), special “dataset-path mode” activates for collection URLs ending in/dataset, bypassing the catalog to load only the selected dataset details (and using generated HTML as fallback for deep links).--data-urldocumentation for runtime behavior, caching/error expectations, CORS guidance, and help text examples.