-
Notifications
You must be signed in to change notification settings - Fork 13
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
[Is a member of][User groups] 'Add' button functionality #298
Conversation
4dd7c04
to
0338952
Compare
@pvoborni - The code has been corrected based on your feedback. |
4e8b00b
to
36d34ea
Compare
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.
Next round of review finished.
Besides @pvoborni comments, LGTM |
@pvoborni - There is one comment of yours that refers to outdated code in addToGroups: build.mutation<BatchRPCResponse, [string, string, string[]]>({
query: (payload) => {
const toId = payload[0];
const type = payload[1];
const listOfMembers = payload[2];
const membersToAdd: Command[] = [];
listOfMembers.map((member) => {
const payloadItem = {
method: "group_add_member",
params: [[member], { [type]: toId }],
} as Command;
membersToAdd.push(payloadItem);
});
return getBatchCommand(membersToAdd, API_VERSION_BACKUP);
},
}), Basically, I added a third parameter that refers to the |
dd7195f
to
00dabd5
Compare
The comment might have been attached to outdate code (not sure why), but it was about this implementation and still applies fully. |
The 'Add' button functionality allows to associate any user group's member to a given user. Signed-off-by: Carla Martinez <[email protected]>
00dabd5
to
321b371
Compare
Understood. I have already corrected the code. |
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.
LGTM
The 'Add' button functionality allows associating any user group's member to a given user.
To ensure that an Alert component will be displayed after a new addition, a new interface AlertObject has been created to refer to a given alert (
const alerts = useAlerts()
) passed through props to a children component.This PR depends on this one to be merged: #297