Skip to content

Conversation

aiAdrian
Copy link
Contributor

@aiAdrian aiAdrian commented Oct 1, 2025

Description

Issues

Checklist

  • This PR contains a description of the changes I'm making
  • I've read the Contribution Guidelines
  • I've added tests for changes or features I've introduced
  • I documented any high-level concepts I'm introducing in documentation/
  • CI is currently green and this is ready for review

@aiAdrian aiAdrian added this to the 2.10 milestone Oct 1, 2025
@aiAdrian aiAdrian self-assigned this Oct 1, 2025
@aiAdrian aiAdrian added the bug Something isn't working label Oct 1, 2025
@aiAdrian aiAdrian linked an issue Oct 1, 2025 that may be closed by this pull request
3 tasks
if (confirmed) {
this.nodeService.deleteNode(node.getId());
this.nodeDeleted.emit();
this.nodeService.deleteNode(node.getId());
Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer that the very last thing that is done is the this.nodeDeleted.emit(), otherwise we have have an emit and then have an error (and so still emit, which would be wrong)

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the this.nodeService.deleteNode(node.getId()); line should not be moved

@aiAdrian aiAdrian added the help wanted Extra attention is needed label Oct 6, 2025
Copy link
Contributor

@louisgreiner louisgreiner left a comment

Choose a reason for hiding this comment

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

@louisgreiner
Copy link
Contributor

Close #544

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working help wanted Extra attention is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: delete node from side view result on an error

2 participants