Skip to content
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

Merged
merged 5 commits into from
Jun 24, 2024

Conversation

maryarm
Copy link
Contributor

@maryarm maryarm commented Jun 21, 2024

Description

Identity UI support list/create/edit/delete on Role.

Related issues

Part of identity issue (#2818)

@github-actions github-actions bot added the component/identity Related to the Identity component/team label Jun 21, 2024
@maryarm maryarm requested a review from dlavrenuek June 21, 2024 12:48
Copy link
Contributor

@dlavrenuek dlavrenuek left a 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?

Comment on lines 1 to 7
/*
* @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.
*/

Copy link
Contributor

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.

Comment on lines 19 to 50
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,
})}
Copy link
Contributor

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

Suggested change
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,
Copy link
Contributor

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.

@maryarm maryarm requested a review from dlavrenuek June 21, 2024 13:28
Copy link
Contributor

@dlavrenuek dlavrenuek left a 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,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
id: id,
id,

@maryarm maryarm added this pull request to the merge queue Jun 24, 2024
Merged via the queue into main with commit 1d23b16 Jun 24, 2024
25 checks passed
@maryarm maryarm deleted the identity/2818-roles-ui branch June 24, 2024 08:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/identity Related to the Identity component/team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants