Skip to content

Cb 43 refresh data#63

Merged
dkrupenya merged 7 commits intodevelfrom
CB-43-refresh-data
May 28, 2020
Merged

Cb 43 refresh data#63
dkrupenya merged 7 commits intodevelfrom
CB-43-refresh-data

Conversation

@dkrupenya
Copy link
Copy Markdown
Contributor

No description provided.

@dkrupenya dkrupenya requested a review from Wroud May 28, 2020 09:04
isPresent(context) {
return context.contextType === NavigationTreeContextMenuService.nodeContextType;
},
order: 1000,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

better to use Number.MAX_SAFE_INTEGER

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.

fixed

}

async refreshNode(nodeId: string) {
console.log('refresh', nodeId);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

remove?

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.

fixed

Comment on lines +86 to +87
this.nodesStore.removeNode(nodeId);
await this.updateNodeInfo(nodeId);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

remove can cause many bugs in case of an update, so we probably must check the result of request inside updateNodeInfo and remove node if it does not exist anymore, on other cases we probably just update node related data

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.

to be done later

@dkrupenya dkrupenya merged commit a33608a into devel May 28, 2020
@Wroud Wroud deleted the CB-43-refresh-data branch June 6, 2020 17:25
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.

2 participants