Skip to content

fix: remove sketch file reference when deleting assets#4161

Open
yugalkaushik wants to merge 2 commits into
processing:developfrom
yugalkaushik:asset-fix
Open

fix: remove sketch file reference when deleting assets#4161
yugalkaushik wants to merge 2 commits into
processing:developfrom
yugalkaushik:asset-fix

Conversation

@yugalkaushik

Copy link
Copy Markdown
Contributor

Issue:

Fixes #4160

Asset deletion now uses the same file delete endpoint as the sketch files sidebar, so both the storage object and the project file reference are removed together.

Before

issue-editor.mp4

After

pr-preview.mp4

Changes:

  1. Include fileId and parentId in the asset list API response when matching assets to sketch files
  2. Update asset deletion to call /projects/:sketchId/files/:fileId instead of /S3/delete
  3. Sync Redux state when the currently open sketch matches the deleted asset

I have verified that this pull request:

  • has no linting errors (npm run lint)
  • has no test errors (npm run test)
  • has no typecheck errors (npm run typecheck)
  • is from a uniquely-named feature branch and is up to date with the develop branch.
  • is descriptively named and links to an issue number, i.e. Fixes #123
  • meets the standards outlined in the accessibility guidelines

Comment thread client/modules/IDE/actions/assets.js Outdated
Comment on lines +39 to +48

const { project } = getState();
if (project.id === asset.sketchId) {
dispatch(setProjectSavedTime(response.data.project.updatedAt));
dispatch({
type: ActionTypes.DELETE_FILE,
id: asset.fileId,
parentId: asset.parentId
});
}

@skyash-dev skyash-dev Jun 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can remove this. unlike deleteFile, there's no need to reactively update local state here since the user is on a different screen, and returning to the sketch will load the latest data anyway.

edit: also getState() returns the currently open project state, which may not even be the project associated with the asset being deleted.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding getState, it's still present solely for the DELETE_FILE dispatch, which is guarded by if (project.id === asset.sketchId). This ensures that when a user deletes a referenced asset from the Assets tab while on the same sketch, the file tree sidebar updates immediately without requiring a page refresh. When the user is viewing a different sketch, the guard prevents any action. So getState is safe and necessary for this reactive UI update.

Comment thread client/modules/IDE/actions/assets.js
Comment thread client/modules/IDE/components/AssetListRow.jsx Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Deleting an asset leaves a broken file reference in sketch files

2 participants