Dashboard: move a widget from one dashboard to another #2923
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -130,6 +130,64 @@ class EditDashboard extends Component { | |
| this.setState(newState); | ||
| }; | ||
|
|
||
| moveBoxToDashboard = async (x, y, dashboardSelector) => { | ||
| const box = get(this.state, `currentDashboard.boxes.${x}.${y}`); | ||
| // We don't move a box which is not configured yet | ||
| if (!box || box.type === undefined) { | ||
| return; | ||
| } | ||
| // The box is removed from the current dashboard, and saved in a list of boxes | ||
| // to move when the user will save the dashboard. | ||
| const newState = update(this.state, { | ||
| currentDashboard: { | ||
| boxes: { | ||
| [x]: { | ||
| $splice: [[y, 1]] | ||
| } | ||
| } | ||
| }, | ||
| boxesToMove: { | ||
| $push: [{ dashboardSelector, box }] | ||
| } | ||
| }); | ||
| await this.setState({ ...newState, boxNotEmptyError: false }); | ||
| }; | ||
|
|
||
| moveBoxesToOtherDashboards = async () => { | ||
| const { boxesToMove } = this.state; | ||
| if (boxesToMove.length === 0) { | ||
| return; | ||
| } | ||
| // Boxes are grouped by destination dashboard, so each dashboard is updated only once | ||
| const boxesByDashboard = {}; | ||
| boxesToMove.forEach(boxToMove => { | ||
| boxesByDashboard[boxToMove.dashboardSelector] = (boxesByDashboard[boxToMove.dashboardSelector] || []).concat([ | ||
| boxToMove.box | ||
| ]); | ||
| }); | ||
| await Promise.all( | ||
| Object.keys(boxesByDashboard).map(async dashboardSelector => { | ||
| const dashboard = await this.props.httpClient.get(`/api/v1/dashboard/${dashboardSelector}`); | ||
| const columns = dashboard.boxes && dashboard.boxes.length > 0 ? dashboard.boxes : [[]]; | ||
| // Boxes are added at the end of the first column of the destination dashboard | ||
| const newColumns = update(columns, { | ||
| 0: { | ||
| $push: boxesByDashboard[dashboardSelector] | ||
| } | ||
| }); | ||
| await this.props.httpClient.patch(`/api/v1/dashboard/${dashboardSelector}`, { | ||
| ...dashboard, | ||
| boxes: newColumns | ||
| }); | ||
| // Boxes are removed from the pending list as soon as they are saved, | ||
| // so they are not moved twice if the user retries after an error. | ||
| this.setState(prevState => ({ | ||
| boxesToMove: prevState.boxesToMove.filter(boxToMove => boxToMove.dashboardSelector !== dashboardSelector) | ||
| })); | ||
|
Comment on lines
+159
to
+189
There was a problem hiding this comment. 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift Make destination updates idempotent. Line 178 appends boxes and sends a non-idempotent patch. If the server applies the patch but the client loses the response, the request rejects and the queue remains. A user retry fetches the updated destination and appends the same box again. Use a server-deduplicated move operation ID, or a versioned conditional update with durable deduplication. Do not rely only on a resolved client response to remove a queued move. 🤖 Prompt for AI Agents
Contributor
Author
There was a problem hiding this comment. Declining this one, as out of scope for this PR. The suggested remedies — a server-deduplicated move operation ID, or a versioned conditional update with durable deduplication — both require a new server-side contract. This PR is deliberately front-only: it reuses the existing The residual window is also narrow and not specific to this feature: it needs a PATCH to succeed while its response is lost, followed by a user retry. The same read-modify-write non-idempotency already exists in the normal dashboard save path, so addressing it properly belongs in a dedicated change to the dashboard API rather than here. What this PR does guarantee is the more damaging failure mode: destination dashboards are patched before the source dashboard is saved, and each box is dropped from the pending list as soon as its destination write resolves, so a widget is never lost and a successful move is never replayed. Generated by Claude Code There was a problem hiding this comment.
The current ordering protects against widget loss during a source-dashboard save failure. The duplicate risk requires an ambiguous destination Would you like me to create a follow-up GitHub issue for an idempotent dashboard move API? 🐇 ✏️ Learnings added
You are interacting with an AI system. |
||
| }) | ||
| ); | ||
|
cursor[bot] marked this conversation as resolved.
|
||
| }; | ||
|
|
||
| removeBox = async (x, y) => { | ||
| const newState = update(this.state, { | ||
| currentDashboard: { | ||
|
|
@@ -241,6 +299,10 @@ class EditDashboard extends Component { | |
| // We purge all empty boxes | ||
| await this.removeEmptyBoxes(); | ||
|
|
||
| // Boxes moved to another dashboard are saved in those dashboards first, | ||
| // so a box is never lost if this dashboard fails to save. | ||
| await this.moveBoxesToOtherDashboards(); | ||
|
|
||
| const { currentDashboard: selectedDashboard, dashboards } = this.state; | ||
| const { selector } = selectedDashboard; | ||
|
|
||
|
|
@@ -264,6 +326,8 @@ class EditDashboard extends Component { | |
| route(`/dashboard/${currentDashboard.selector}`); | ||
| } catch (e) { | ||
| console.error(e); | ||
| // The save failed, we stop the loader so the user can fix the error and retry | ||
| this.setState({ loading: false }); | ||
| if (e.response && e.response.status === 422) { | ||
| this.setState({ | ||
| dashboardValidationError: true | ||
|
|
@@ -372,7 +436,8 @@ class EditDashboard extends Component { | |
| askDeleteDashboard: false, | ||
| boxNotEmptyError: false, | ||
| columnBoxNotEmptyError: null, | ||
| isMobileReordering: false, | ||
| boxesToMove: [] | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| }; | ||
| } | ||
|
|
||
|
|
@@ -399,7 +464,8 @@ class EditDashboard extends Component { | |
| boxNotEmptyError, | ||
| columnBoxNotEmptyError, | ||
| savingNewDashboardList, | ||
| isMobileReordering, | ||
| boxesToMove | ||
| } | ||
| ) { | ||
| return ( | ||
|
|
@@ -421,6 +487,8 @@ class EditDashboard extends Component { | |
| addBox={this.addBox} | ||
| addBoxAtPosition={this.addBoxAtPosition} | ||
| removeBox={this.removeBox} | ||
| moveBoxToDashboard={this.moveBoxToDashboard} | ||
| boxesToMove={boxesToMove} | ||
| updateNewSelectedBox={this.updateNewSelectedBox} | ||
| saveDashboard={this.saveDashboard} | ||
| updateBoxConfig={this.updateBoxConfig} | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.