Accept a role ID on linking an account to LDAP #8236
Conversation
boring-cyborg
bot
added
component:integration-test
component:LDAP
Python
|
@blueorangutan package |
|
@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #8236 +/- ##
=============================================
+ Coverage 18.28% 30.76% +12.48%
- Complexity 16826 33931 +17105
=============================================
Files 4848 5341 +493
Lines 324301 374976 +50675
Branches 45561 54546 +8985
=============================================
+ Hits 59290 115363 +56073
+ Misses 256437 244331 -12106
- Partials 8574 15282 +6708
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
@blueorangutan test keepEnv |
|
@DaanHoogland a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
[SF] Trillian test result (tid-8330)
|
DaanHoogland
force-pushed
the
linkLdapToRoleType
branch
from
4d1783f
to
43a64b7
Compare
DaanHoogland
force-pushed
the
linkLdapToRoleType
branch
from
43a64b7
to
f6b76f3
Compare
|
@blueorangutan package |
|
@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8345 |
|
@blueorangutan test |
|
@DaanHoogland a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
[SF] Trillian test result (tid-8864)
|
|
@JoaoJandre , this could also go into 4.18.2. borderline enhancement. |
Sure, I don't see a problem with it |
/ok, I'll rebase on 4.18/won't backport as 4.18.2 is out already/ |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 4.19 #8236 +/- ##
============================================
- Coverage 14.96% 14.95% -0.01%
+ Complexity 10995 10987 -8
============================================
Files 5373 5373
Lines 468989 469171 +182
Branches 61009 60977 -32
============================================
- Hits 70191 70182 -9
- Misses 391019 391221 +202
+ Partials 7779 7768 -11
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
DaanHoogland
force-pushed
the
linkLdapToRoleType
branch
from
f6b76f3
to
e1e6a90
Compare
|
@blueorangutan package |
|
@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 9445 |
|
@blueorangutan test |
|
@DaanHoogland a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
[SF] Trillian test result (tid-10053)
|
| @@ -134,7 +138,14 @@ public String getAdmin() { | |||
| } | |||
|
|
|||
| public Account.Type getAccountType() { | |||
| return Account.Type.getFromValue(accountType); | |||
| if (accountType == null) { | |||
| return RoleType.getAccountTypeByRole(roleService.findRole(roleId), null); | |||
There was a problem hiding this comment.
@DaanHoogland This will return null if roleId is null. Is that fine?
@DaanHoogland Do we need some validations around input as well? e.g. both account type & role id can't be null.
| } | ||
|
|
||
| public Long getRoleId() { | ||
| return RoleType.getRoleByAccountType(roleId, getAccountType()); |
There was a problem hiding this comment.
This can be null as well. Is that fine?
Description
This PR implements #3677
Fixes: #3677
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?