-
Notifications
You must be signed in to change notification settings - Fork 563
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
feat: add list, create, edit, delete roles to Identity UI #19637
Conversation
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.
One code style suggestion from my side, looks good otherwise?
/* | ||
* @license Identity | ||
* Copyright Camunda Services GmbH and/or licensed to Camunda Services GmbH under one or more contributor license | ||
* agreements. Licensed under a proprietary license. See the License.txt file for more information. You may not use this | ||
* file except in compliance with the proprietary license. | ||
*/ | ||
|
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.
The license header is from old Identity. We can delete if for now or include the new license header.
entity: role, | ||
open, | ||
onClose, | ||
onSuccess, | ||
}) => { | ||
const { t } = useTranslate(); | ||
const { enqueueNotification } = useNotifications(); | ||
|
||
const [callDeleteApi, { loading }] = useApiCall(deleteRole); | ||
|
||
const handleSubmit = async () => { | ||
if (role) { | ||
const { success } = await callDeleteApi({ | ||
id: role.id, | ||
}); | ||
|
||
if (success) { | ||
enqueueNotification({ | ||
kind: "success", | ||
title: t("Role has been deleted."), | ||
}); | ||
onSuccess(); | ||
} | ||
} | ||
}; | ||
|
||
return ( | ||
<Modal | ||
open={open} | ||
headline={t('Are you sure you want to delete the role "{{ name }}"?', { | ||
name: role?.name, | ||
})} |
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.
Descructuring role
to { id, name }
can make the code look a bit better
entity: role, | |
open, | |
onClose, | |
onSuccess, | |
}) => { | |
const { t } = useTranslate(); | |
const { enqueueNotification } = useNotifications(); | |
const [callDeleteApi, { loading }] = useApiCall(deleteRole); | |
const handleSubmit = async () => { | |
if (role) { | |
const { success } = await callDeleteApi({ | |
id: role.id, | |
}); | |
if (success) { | |
enqueueNotification({ | |
kind: "success", | |
title: t("Role has been deleted."), | |
}); | |
onSuccess(); | |
} | |
} | |
}; | |
return ( | |
<Modal | |
open={open} | |
headline={t('Are you sure you want to delete the role "{{ name }}"?', { | |
name: role?.name, | |
})} | |
entity: { id, name }, | |
open, | |
onClose, | |
onSuccess, | |
}) => { | |
const { t } = useTranslate(); | |
const { enqueueNotification } = useNotifications(); | |
const [callDeleteApi, { loading }] = useApiCall(deleteRole); | |
const handleSubmit = async () => { | |
if (role) { | |
const { success } = await callDeleteApi({ id }); | |
if (success) { | |
enqueueNotification({ | |
kind: "success", | |
title: t("Role has been deleted."), | |
}); | |
onSuccess(); | |
} | |
} | |
}; | |
return ( | |
<Modal | |
open={open} | |
headline={t('Are you sure you want to delete the role "{{ name }}"?', { | |
name, | |
})} |
import usePermissions from "src/pages/roles/modals/usePermissions"; | ||
|
||
const EditModal: FC<UseEntityModalProps<Role>> = ({ | ||
entity: role, |
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.
FYI: I did not use desctucturing here because the variables name
, description
and permissions
already exist, so you would need to name it like currentName
etc... which in the end is longer than using role.name.
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.
One last suggestion from my side to make it perfect, but can also be merged as it is 🏅
id: role.id, | ||
}); | ||
const { success } = await callDeleteApi({ | ||
id: id, |
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.
id: id, | |
id, |
Description
Identity UI support list/create/edit/delete on Role.
Related issues
Part of identity issue (#2818)