-
Notifications
You must be signed in to change notification settings - Fork 810
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
fix(core): prevent app permissions from being lost during an app upse… #2217
Conversation
@@ -44,10 +45,15 @@ class UpsertApplicationTask extends AbstractFront50Task { | |||
} else { | |||
log.info("Creating application (name: ${application.name})") | |||
front50Service.create(application) | |||
if (application?.permission?.permissions == null) { |
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.
You can probably groovify this with if (!application?.permission?.permissions)
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.
I want this condition to be true for an empty map - null
should mean no change to the permissions, whereas [:]
should mean an empty set of permissions.
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.
It'll also be true if application
or permission
are null
when written like this -- is that intended?
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.
Yes for permission
, no for application
, so I'll take out the null check for application.
} | ||
|
||
try { | ||
front50Service.updatePermission(application.name, application.permission) | ||
if (application?.permission?.permissions != null) { |
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.
Similar here to above
548ba3d
to
bf987e9
Compare
bf987e9
to
51080b3
Compare
LGTM |
…rt operation