feat: Add dashboard duplication feature
Implements the ability to duplicate dashboards from the options dropdown menu.
Closes #1253
Changes
- Backend: Add duplicateDashboard controller function
- Backend: Add POST /dashboards/:id/duplicate API endpoint
- Frontend: Add useDuplicateDashboard hook
- Frontend: Add "Duplicate Dashboard" option to menu dropdown
- Tests: Add comprehensive test coverage for duplication functionality
Requirements Met
- ✅ Alerts are NOT copied to the duplicated dashboard
- ✅ All tile IDs are regenerated to be unique
- ✅ Dashboard name gets " (Copy)" suffix
- ✅ All tiles, tags, and filters are copied
- ✅ Comprehensive test coverage added
Generated with Claude Code
🦋 Changeset detected
Latest commit: 630510df68a1c632f236406c451371982c045ee4
The changes in this PR will be included in the next version bump.
This PR includes changesets to release 2 packages
| Name | Type |
|---|---|
| @hyperdx/api | Minor |
| @hyperdx/app | Minor |
Not sure what this means? Click here to learn what changesets are.
Click here if you're a maintainer who wants to add another changeset to this PR
The latest updates on your projects. Learn more about Vercel for GitHub.
| Project | Deployment | Preview | Comments | Updated (UTC) |
|---|---|---|---|---|
| hyperdx-v2-oss-app | Preview | Comment | Oct 14, 2025 9:21pm |
Pull Request Review: Dashboard Duplication Feature
Thank you for implementing this feature! The implementation is generally solid and well-tested. Below are my findings:
Strengths
- Excellent Test Coverage - comprehensive tests covering all critical scenarios
- Clear Implementation - follows existing patterns in the codebase
- Good UX - frontend integration with notifications and navigation
- Requirement Compliance - all stated requirements are met
Critical Issues
1. Weak Tile ID Generation Algorithm (dashboard.ts:206)
The current tile ID generation using Math.random().toString(36) has collision risks. With Math.random(), collision probability increases with more tiles and there is no guarantee of uniqueness across the system.
Recommendation: Use crypto.randomBytes() or MongoDB ObjectId for more robust ID generation. Consider creating a shared utility function in packages/common-utils.
Medium Priority Issues
2. Inconsistent Error Handling (dashboards.ts:123)
The route handler checks if the dashboard exists twice - once in the router and once in the controller. Remove the redundant check.
3. Tile Data Structure Spread (dashboard.ts:204-212)
Using spread operator ...tile may inadvertently copy Mongoose document metadata. Use toObject() before spreading.
4. Missing Team-Based Access Control Test
Tests don't verify that users can't duplicate dashboards from other teams. Add a test case for team isolation.
Minor Suggestions
- Unused userId parameter - consider removing unless planned for audit logging
- Consider adding logging for dashboard duplication events
- Frontend error handling could be more specific based on status codes
- Add JSDoc comments explaining duplication behavior
Security
No security concerns identified. Implementation properly uses team-based access control, validates input with Zod schemas, and follows authentication middleware patterns.
Summary
Overall Assessment: Good implementation with solid test coverage. Main concern is the tile ID generation algorithm.
Recommendation: Address the tile ID generation issue before merging. Other issues can be addressed in follow-up PRs.
Estimated Risk: Low-Medium (due to potential tile ID collisions)
E2E Test Results
✅ All tests passed • 25 passed • 3 skipped • 223s
| Status | Count |
|---|---|
| ✅ Passed | 25 |
| ❌ Failed | 0 |
| ⚠️ Flaky | 0 |
| ⏭️ Skipped | 3 |
@claude run yarn run lint from the main directory to run the linter. Implement the fixes needed so that linting passes
PR Review: Dashboard Duplication Feature
Thanks for implementing this feature! Overall, the implementation is solid and well-tested. Here's my feedback:
✅ Strengths
- Excellent test coverage - Comprehensive tests cover all requirements including unique tile IDs, alert exclusion, and filter preservation
- Clear implementation - The code is straightforward and easy to understand
- Proper multi-tenancy - Correctly enforces team-based access control
- Good UX - Includes proper notifications and navigation after duplication
- Follows conventions - Uses established patterns from the codebase (TanStack Query hooks, Mantine UI components)
🔍 Issues & Recommendations
1. Tile ID Generation Consistency (Minor)
Location: packages/api/src/controllers/dashboard.ts:206
The tile ID generation uses:
id: Math.floor(100000000 * Math.random()).toString(36)
This matches the frontend pattern in DBDashboardPage.tsx:94. However, I noticed potential collision risks:
-
Math.random()is not cryptographically secure - The range
100000000 * Math.random()could theoretically produce duplicates
Recommendation: Consider using a more robust ID generation approach:
import { randomBytes } from 'crypto';
// Option 1: Use crypto module
id: randomBytes(8).toString('hex')
// Option 2: Use ObjectId-style generation
id: new mongoose.Types.ObjectId().toString()
While collision probability is low with the current approach, using a cryptographically secure method would be more robust for production.
2. getDashboard Return Type Inconsistency (Minor)
Location: packages/api/src/routers/api/dashboards.ts:121
The duplicate endpoint calls getDashboard() which returns an object with spread tiles (not a Mongoose document), but then immediately calls duplicateDashboard() which queries the database again. This is redundant.
Current code:
const dashboard = await getDashboard(dashboardId, teamId);
if (dashboard == null) {
return res.sendStatus(404);
}
const newDashboard = await duplicateDashboard(dashboardId, teamId, userId);
Recommendation: Remove the redundant getDashboard call since duplicateDashboard already checks for dashboard existence:
const newDashboard = await duplicateDashboard(dashboardId, teamId, userId);
if (newDashboard == null) {
return res.sendStatus(404);
}
res.json(newDashboard.toJSON());
Then update the controller to return null instead of throwing:
export async function duplicateDashboard(...) {
const dashboard = await Dashboard.findOne({
_id: dashboardId,
team: teamId,
});
if (dashboard == null) {
return null; // Instead of throwing
}
// ... rest of implementation
}
3. Alert Removal Implementation (Observation)
Location: packages/api/src/controllers/dashboard.ts:208-211
The current implementation manually removes alerts from tile configs:
config: {
...tile.config,
alert: undefined,
}
This works correctly, but consider documenting WHY alerts aren't copied. The comment says "per requirement" but future developers might wonder about the business logic.
Recommendation: Add a more descriptive comment:
// Alerts are intentionally excluded from duplicated dashboards to prevent
// unintended notification spam. Users must explicitly configure alerts
// for the new dashboard if needed.
config: {
...tile.config,
alert: undefined,
}
4. Error Handling in Frontend (Minor)
Location: packages/app/src/DBDashboardPage.tsx:983-999
The error notification shows a generic message. Consider including the actual error message for better debugging:
onError: (error) => {
notifications.show({
color: 'red',
title: 'Failed to duplicate dashboard',
message: error?.message || 'An error occurred while duplicating the dashboard',
});
}
🎯 Minor Observations
-
Unused parameter: The
_userIdparameter induplicateDashboardis prefixed with underscore (correct) but never used. Consider if there's a need to track who duplicated the dashboard for audit purposes. -
Changeset description: The changeset says "dashboard clone" but the PR calls it "duplicate" - minor terminology inconsistency (not a blocker).
🔒 Security
✅ No security concerns. The implementation properly:
- Enforces team-based access control
- Validates dashboard ownership before duplication
- Uses Zod validation for request parameters
📊 Performance
✅ No performance concerns. The operation is:
- Single database write for the new dashboard
- No alert creation overhead
- Properly invalidates React Query cache
🎨 Code Style
✅ Follows HyperDX conventions:
- Uses Mantine UI components (per CLAUDE.md guidelines)
- Follows MongoDB model patterns
- Uses TanStack Query for server state
- Proper TypeScript typing
📝 Verdict
Approve with minor suggestions. The core implementation is solid and meets all requirements. The issues mentioned above are minor improvements that could be addressed in a follow-up or before merging, but they don't block the PR.
Great work on the comprehensive test coverage! 🎉