fix(observability): duplicated dashboard cannot be deleted

[cnoramut]

Description

Duplicating an Observability Dashboard produced a copy that could not be deleted. Selecting either row selected all of them, deleting the copy deleted the original instead, and a second delete attempt failed with Error deleting Observability Dashboards, please make sure you have the correct permission.

Root cause. onClone built the new panel from the source with its id intact, so the source id was persisted as a saved object attribute. savedObjectToCustomPanel then spread those attributes over the real saved object id.

 const savedObjectToCustomPanel = (so: SimpleSavedObject<PanelType>): CustomPanelType => ({
-  id: so.id,                          // a stale `id` in attributes wins over this
-  type: so.type,
-  objectId: so.type + ':' + so.id,
-  ...so.attributes,
+  ...so.attributes,
+  type: so.type,
+  objectId: so.type + ':' + so.id,
+  id: so.id,                          // spread first, so the real id always wins
   savedObject: true,
 });

So every copy took the original's id as its row id. The table keys selection on that id, hence one click selecting all copies. Delete then targeted the original, and the follow-up delete hit a document that no longer existed, which the catch block reports as a permission error.

The stale id is not a uuid, so isUuid routed copies down the legacy branch. Editing a copy posted its updates to the original, and duplicating navigated to the original rather than the new copy.

Fix. onClone now dispatches the existing clonePanel thunk, which already strips the id and is what both panel view pages use. savedObjectToCustomPanel spreads attributes first so type, objectId, and id are authoritative.

Notes for reviewers

1. Two regression tests ship with this. Both were verified red against the pre-fix code and green after. One asserts the clone payload carries no id, the other that a stale id in attributes cannot shadow the real saved object id.
2. Copies now get their own timestamps. clonePanel sets dateCreated and dateModified, which the previous table path did not, so a duplicate no longer inherits the original's Last updated value in a table sorted on that column.

Before

Before.2477.mov

After

After.2477.mov

Issues Resolved

Fixes #2477

Check List

• New functionality includes testing.
  • All tests pass, including unit test, integration test and doctest
• New functionality has been documented.
  • New functionality has javadoc added
  • New functionality has user manual doc added
• Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 390b34a)

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ Recommended focus areas for review Unused import createPanel is no longer used after switching onClone to dispatch clonePanel, but it remains in the import list. Verify whether createPanel is still referenced elsewhere in this file; if not, remove it to avoid a lint/unused-import warning. import { clonePanel, createPanel, deletePanels, fetchPanels, isUuid,

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 390b34a
Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Ensure clone strips source identifiers The diff removes the clonePanel implementation reference and imports it in custom_panel_table.tsx, but the definition is not visible in this diff. Ensure clonePanel is exported from panel_slice.ts and strips the source id/objectId before calling savedObjectsClient.create, otherwise the fix (preventing the stale id from being copied) is incomplete and the test in custom_panel_table.test.tsx asserting attributes has no id will fail. public/components/custom_panels/redux/panel_slice.ts [356-363] -const savedObjectToCustomPanel = (so: SimpleSavedObject<PanelType>): CustomPanelType => ({ - ...so.attributes, - // Spread first so stale copies of these in attributes cannot shadow the real values. - type: so.type, - objectId: so.type + ':' + so.id, - id: so.id, - savedObject: true, -}); +export const clonePanel = (sourcePanel: CustomPanelType, newName: string) => async (dispatch) => { + const { id: _id, objectId: _objectId, ...rest } = sourcePanel; + const newPanel = { ...rest, title: newName }; + dispatch(createPanel(newPanel)); +}; Suggestion importance[1-10]: 6 __ Why: The suggestion raises a valid concern about clonePanel needing to strip the source id/objectId to make the new test pass, but since the actual clonePanel implementation is outside this diff, the suggestion is speculative and asks the user to verify. The improved code is a reasonable implementation but cannot be confirmed correct against the actual definition.	Low

---

Previous suggestions

Suggestions up to commit 25d03ae

Category	Suggestion	Impact
General	Harden against stale id in attributes The fix relies on this ordering to prevent a stale id in so.attributes from overriding the real saved-object id. Add an inline eslint-disable or explicit comment to prevent future refactors (e.g., reformatting/auto-fix) from re-ordering the spread and reintroducing the bug. Consider also explicitly deleting the stale id after spread as a defense-in-depth. public/components/custom_panels/redux/panel_slice.ts [356-363] -const savedObjectToCustomPanel = (so: SimpleSavedObject<PanelType>): CustomPanelType => ({ - ...so.attributes, - // Spread first so stale copies of these in attributes cannot shadow the real values. - type: so.type, - objectId: so.type + ':' + so.id, - id: so.id, - savedObject: true, -}); +const savedObjectToCustomPanel = (so: SimpleSavedObject<PanelType>): CustomPanelType => { + // Note: spread MUST come before id/type/objectId so stale copies in attributes cannot shadow real values. + const { id: _staleId, ...cleanAttributes } = so.attributes as any; + return { + ...cleanAttributes, + type: so.type, + objectId: so.type + ':' + so.id, + id: so.id, + savedObject: true, + }; +}; Suggestion importance[1-10]: 6 __ Why: The suggestion adds defense-in-depth by explicitly stripping the stale id from attributes before spreading, which is more robust than relying solely on property ordering. This is a reasonable hardening improvement, though the existing code with the comment already addresses the bug.	Low

[github-actions]

Persistent review updated to latest commit 390b34a