Repository navigation
Update Header Menu and add duplicate feature - #123
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a tacticboard “duplicate” feature (API + UI) and consolidates several header actions into a shared overflow menu component across multiple pages.
Changes:
- Added
POST /api/tacticboards/:id/duplicatebackend endpoint with access checks and a contract test. - Added
HeaderOverflowMenuUI component and migrated header actions in TacticBoardProfile, PracticePlanner, and Exercise pages into the overflow menu. - Added client RTK Query mutation + i18n strings for the new menu and duplication flow.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| server/controllers/tacticboardController.ts | Implements duplication endpoint and related authorization checks. |
| server/routes/tacticboardRoutes.ts | Registers the new /duplicate route. |
| server/tests/contract/tacticboards.duplicate.spec.ts | Adds a contract test for tacticboard duplication. |
| client/src/api/quadcoachApi/tacticboardApi.ts | Adds duplicateTacticBoard mutation and export hook. |
| client/src/components/HeaderOverflowMenu.tsx | New reusable overflow menu component for header actions. |
| client/src/components/index.ts | Re-exports the new menu component and action type. |
| client/src/pages/TacticBoardProfile/TacticBoardProfile.tsx | Uses overflow menu + adds duplicate action and error handling. |
| client/src/pages/TacticBoardProfile/translations/TacticBoardProfile_en.json | Adds menu strings + duplication error message. |
| client/src/pages/TacticBoardProfile/translations/TacticBoardProfile_de.json | Adds menu strings + duplication error message (DE). |
| client/src/pages/PracticePlanner/PracticePlanner.tsx | Migrates share/delete header actions into overflow menu. |
| client/src/pages/PracticePlanner/translations/PracticePlanner_en.json | Adds menu strings. |
| client/src/pages/PracticePlanner/translations/PracticePlanner_de.json | Adds menu strings (DE). |
| client/src/pages/Exercises/Exercise/ViewExercise/Exercise.tsx | Migrates delete action into overflow menu. |
| client/src/pages/Exercises/Exercise/ViewExercise/translations/Exercise_en.json | Adds menu strings + share dialog strings. |
| client/src/pages/Exercises/Exercise/ViewExercise/translations/Exercise_de.json | Adds menu strings + share dialog strings (DE). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const duplicatedTacticboard = new TacticBoard({ | ||
| name: `Copy of ${tacticboard.name ?? "Tacticboard"}`, | ||
| isPrivate: tacticboardData.isPrivate, | ||
| tags: tacticboardData.tags, | ||
| pages: tacticboardData.pages, | ||
| description: tacticboardData.description, | ||
| coaching_points: tacticboardData.coaching_points, | ||
| creator: req.UserInfo.name ?? tacticboardData.creator ?? req.UserInfo.id, | ||
| user: req.UserInfo.id, | ||
| }); |
There was a problem hiding this comment.
creator on the duplicated board can incorrectly fall back to the source board's creator when req.UserInfo.name is missing. That would attribute the copy to the wrong person even though user is set to the duplicating user. Prefer always setting creator based on the authenticated user (e.g., req.UserInfo.name or look up the user by req.UserInfo.id), and do not inherit creator from the source board.
| const tacticboardData = tacticboard.toObject() as { | ||
| isPrivate?: boolean; | ||
| tags?: string[]; | ||
| pages?: unknown[]; | ||
| description?: string; | ||
| coaching_points?: string; | ||
| creator?: string; | ||
| }; | ||
|
|
||
| const duplicatedTacticboard = new TacticBoard({ | ||
| name: `Copy of ${tacticboard.name ?? "Tacticboard"}`, | ||
| isPrivate: tacticboardData.isPrivate, | ||
| tags: tacticboardData.tags, | ||
| pages: tacticboardData.pages, | ||
| description: tacticboardData.description, | ||
| coaching_points: tacticboardData.coaching_points, | ||
| creator: req.UserInfo.name ?? tacticboardData.creator ?? req.UserInfo.id, | ||
| user: req.UserInfo.id, | ||
| }); |
There was a problem hiding this comment.
pages: tacticboardData.pages will copy nested subdocument _ids from the source tacticboard (because toObject() includes them by default). This makes the original and duplicate share the same page IDs, which is surprising and can complicate page-level updates/debugging. Consider deep-cloning pages while stripping _id fields (and any nested _ids) so Mongo/Mongoose generates fresh IDs for the duplicated board’s pages.
There was a problem hiding this comment.
Applied in ef360ab. Added a stripIds recursive helper that walks the pages array and removes _id from every level (page subdocuments, object subdocuments, nested objects, etc.) before passing it to the new TacticBoard. Also fixed creator to always be set from the authenticated user (req.UserInfo.name ?? req.UserInfo.id) rather than inheriting from the source board.
| describe("POST /api/tacticboards/:id/duplicate", () => { | ||
| it("duplicates a tacticboard with a copied name for the authenticated user", async () => { | ||
| const { Authorization, user } = await authHeader(); | ||
|
|
||
| const sourceBoard = await TacticBoard.create({ | ||
| name: "Original Board", | ||
| isPrivate: true, | ||
| tags: ["transition"], | ||
| creator: user.name, | ||
| user: user._id, | ||
| pages: [ | ||
| { | ||
| version: "5.3.0", | ||
| objects: [], | ||
| }, | ||
| ], | ||
| description: "Board description", | ||
| coaching_points: "Board coaching points", | ||
| shareToken: "source-share-token", | ||
| }); | ||
|
|
||
| const res = await request(app) | ||
| .post(`/api/tacticboards/${sourceBoard._id}/duplicate`) | ||
| .set("Authorization", Authorization) | ||
| .send(); | ||
|
|
||
| expect(res.status).toBe(201); |
There was a problem hiding this comment.
This contract test only covers the happy-path duplication by the owner. Since duplicateById adds new auth/authorization branches (401 unauthorized, 403 forbidden for private boards without access, 404 not found, 400 invalid ID), add contract tests for at least the 401 and 403 cases to ensure the access control stays correct over time.
…n in duplicate endpoint Agent-Logs-Url: https://github.com/wienans/quadcoach/sessions/5d640f22-67ae-43ae-bb63-0d1d8b3cc8aa Co-authored-by: wienans <40465543+wienans@users.noreply.github.com>
No description provided.