-
-
Notifications
You must be signed in to change notification settings - Fork 210
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(ui): show association error in association dialog #3802
Conversation
Pull Request Test Coverage Report for Build 9854501189Details
💛 - Coveralls |
allowedAssociation() { | ||
const mainNode = this.node | ||
const targetNode = this.group.target | ||
|
||
if (!mainNode || !targetNode) { | ||
return true | ||
} | ||
|
||
if (mainNode.security !== targetNode.security) { | ||
return `Association not allowed, "${targetNode._name}" security is different from "${mainNode._name}"` | ||
} | ||
|
||
return true | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, use Controller.checkAssociation
to figure out if the association is allowed and if not, why not. Then display a different message for each reason.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The screenshot in the issue was just one example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is that actually exported from safe somewhere? Also is that only available in v13?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea is to make a backend api call, the backend calls driver.controller.checkAssociation and passes the result back.
This requires v13 because the method and AssociationCheckResult was only added there.
Supersided by #3804 |
Fixes #3801