Display only directly assigned roles in user modal #1251
Conversation
|
Use Run test server using develop.opencast.org as backend: Specify a different backend like stable.opencast.org: It may take a few seconds for the interface to spin up. |
|
This pull request is deployed at test.admin-interface.opencast.org/1251/2025-05-15_09-02-05/ . |
snoesberger
force-pushed
the
1239-fix-user-roles-effectiveRoles
branch
from
adbe9a9 to
ef618cc
Compare
Not only assigned but also derived roles were display in the "Roles" tab of the user modal.
snoesberger
force-pushed
the
1239-fix-user-roles-effectiveRoles
branch
from
ef618cc to
8c2c112
Compare
|
Thanks for the review @Arnei. I did address your suggestions and did move the filtering to the UserDetails component. Code is ready for an other review. |
| @@ -58,8 +60,16 @@ const UserDetails: React.FC<{ | |||
| setPage(tabNr); | |||
| }; | |||
|
|
|||
| const handleSubmit = (values: UpdateUser) => { | |||
| dispatch(updateUserDetails({values: values, username: userDetails.username})); | |||
| const handleSubmit = (values: any) => { | |||
There was a problem hiding this comment.
Added type for values in my most recent commits.
As with the user details view, we also need to provide the assignedRoles attribute in the New User dialogue.
Since the new user dialog also uses the SelectContainer, I had to add the assignedRoles attribute to the NewUserWizard as well. Now, creating a new user and assigning roles simultaneously is working again. |
| const handleSubmit = (values: { | ||
| status?: 'uninitialized' | 'loading' | 'succeeded' | 'failed', | ||
| error?: SerializedError | null, | ||
| provider?: string, | ||
| roles?: UserRole[], | ||
| name?: string, | ||
| username: string, | ||
| email?: string, | ||
| manageable?: boolean, | ||
| assignedRoles?: UserRole[], | ||
| password?: string, | ||
| passwordConfirmation?: string, | ||
| }) => { |
There was a problem hiding this comment.
You are specifying way more properties than handleSubmit is using. You only need to specify the properties that are actually used (in this case email, name, username, assignedRoles, password). Specifying more properties may confuse future developers that try figure out what is going on.
Same in NewUserWizard.
There was a problem hiding this comment.
I was not sure about that. I took all the attributes that are part of the initialValues. I thought it is possible to get all these attributes when submitting.
How can I find out which attributes I get from onSubmit then?
There was a problem hiding this comment.
It is not important which values handleSubmit could get from onSubmit, it is important which values it actually makes use of. As a general rule, a function should not define taking a value if that value is never used.
How can I find out which attributes I get from onSubmit then?
You cannot, not really, because Formik is not that keen on type safety. I find it to be good practice to define a complete initial state, so that you can reasonably assume to get an object with the same values in onSubmit.
|
Everything else is looking fine now. |
fix #1239
This fix ensures that only directly assigned roles are displayed in the roles tab of the user modal.