Config/user roles parameter template, userInfo updates and refactoring#724
Config/user roles parameter template, userInfo updates and refactoring#724
Conversation
…ED-Labs/proceed into config/user-parameter-roles-template
This comment has been minimized.
This comment has been minimized.
…onfig/user-parameter-roles-template
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
…onfig/user-parameter-roles-template
This comment has been minimized.
This comment has been minimized.
canptura
changed the title
canptura
changed the title
jjoderis
left a comment
jjoderis
left a comment
There was a problem hiding this comment.
At the moment it looks as if the user has the ability to change the virtual parameters like a users name. Is this intended or should this be changed before this PR is merged?
...ent-system-v2/app/(dashboard)/[environmentId]/machine-config/helpers/configuration-helper.ts
Outdated
Show resolved
Hide resolved
.../(dashboard)/[environmentId]/machine-config/templates/configuration-template-organization.ts
Show resolved
Hide resolved
.../(dashboard)/[environmentId]/machine-config/templates/parameter-template-organizationdata.ts
Outdated
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
...stem-v2/app/(dashboard)/[environmentId]/machine-config/[configId]/shared-parameter-utils.tsx
Outdated
Show resolved
Hide resolved
| displayName: [{ text: `${lastName}, ${firstName}`, language: 'en' }], | ||
| subParameters: [dataParameter, rolesParameter, createTemplateUserInfo(membership.userId)], | ||
| }), | ||
| id: membership.id, |
There was a problem hiding this comment.
Couldn't you just pass this to "defaultParameter" as well? Then you could just return the result instead of spreading it into a new object.
There was a problem hiding this comment.
I don't know if we really want the ID of a parameter settable. For now I intentionally kept it so that always a new ID is generated. I don't know if that makes sense though. It just felt wrong
| // mapping to {role, userData[]} objects | ||
| let roleUsersMapping = await asyncMap(rolesWithMembers, async (role) => ({ | ||
| role: role, | ||
| users: await asyncMap(role.members, async (member) => await getUserById(member.id)), |
There was a problem hiding this comment.
This seems to be a bit redundant since the "members" in "role" contain pretty much all the information that is returned by "getUserById". "getRolesWithMembers is more of a "getRolesWithUsers" if we consider "members" to mean the roleMappings. One thing you might miss from "getUserById" return value might be the phone number but you could potentially add that to the data returned by "getRolesWithMembers" if you need it.
There was a problem hiding this comment.
yes, but since we want to retrieve all userInfo I think using getUserById makes sense. It certainly is a bit redundant but the user-fields returned from getRolesWithMembers are hardcoded whereas getUserById returns all columns of the table. We might think about returning all user-fields with getRolesWithMembers if it doesn't break some stuff.
| displayName: [{ text: e.lastName + ', ' + e.firstName, language: 'en' }], | ||
| origin: 'external', | ||
| }), | ||
| id: roleUsers.role.name + e.id, // hardcoded ID for frontend consistency |
There was a problem hiding this comment.
Couldn't you pass "id" to "defaultParameter" as well?
There was a problem hiding this comment.
currently ID is always overwritten by a new random uuid. As mentioned above I don't know if that makes sense.
| origin: 'external', | ||
| subParameters, | ||
| }), | ||
| id: roleUsers.role.name, // hardcoded ID for frontend consistency |
|
✅ Successfully created Preview Deployment. |
2 participants
This branch introduces two new virtual parameters and various changes in the storage/display of userInfo-Parameters as well as some refactorings.