eclipse-tractusx / portal-frontend

Portal Frontend
Apache License 2.0
4 stars 25 forks source link

fix: change required role for app user role overlay #824

Closed typecastcloud closed 1 month ago

typecastcloud commented 1 month ago

Description

Change of roles corresponding to add/edit app user role overlays.

Why

This is for consistency with backend endpoint. owncompany/users/{companyUserId}/apps/{appId}/roles

Issue

Refs: #822

Checklist

Note

Edits by maintainers allowed:

image
typecastcloud commented 1 month ago

Nice - however I suggest to add the permission validation on button level as well. Button should be deactivated or hidden if the permission is not available

I think the logic for that already exists. Please confirm @oyo. Since I only changed the linked role to overlay. I did not test this in my local environment because I have some issues with app setup steps.

image image
oyo commented 1 month ago

Nice - however I suggest to add the permission validation on button level as well. Button should be deactivated or hidden if the permission is not available

I think the logic for that already exists. Please confirm @oyo. Since I only changed the linked role to overlay. I did not test this in my local environment because I have some issues with app setup steps.

@typecastcloud It's not implemented: https://github.com/eclipse-tractusx/portal-frontend/blob/release/v2.0.0-RC9/src/components/pages/AppUserManagement/components/AppUserDetailsTable/index.tsx#L51

Currently the button is only disabled if there are no roles available.

-       addButtonDisabled={roles && roles?.length <= 0}

This looks a little odd - better and also disable if the active user doesn't have the role change to

+       addButtonDisabled={
+         !roles ||
+         roles.length === 0 ||
+         !UserService.hasRole(ROLES.MODIFY_USER_ACCOUNT)
+       }
sonarcloud[bot] commented 1 month ago

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

oyo commented 1 month ago

But on the FE side there is the "Edit" button as well. The same function is needed for this button (and only for this specific page) as well. Please ensure that this has no impacts on any other "Edit" buttons!! :)

@jjeroch updated the PR - are we happy now?

jjeroch commented 1 month ago

But on the FE side there is the "Edit" button as well. The same function is needed for this button (and only for this specific page) as well. Please ensure that this has no impacts on any other "Edit" buttons!! :)

@jjeroch updated the PR - are we happy now?

we are happy now ;)