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: update description on project deletion #7539

Merged
merged 3 commits into from
Jul 5, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ import { formatUnknownError } from 'utils/formatUnknownError';
import useProjectApi from 'hooks/api/actions/useProjectApi/useProjectApi';
import useProjects from 'hooks/api/getters/useProjects/useProjects';
import useToast from 'hooks/useToast';
import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender';
import { useUiFlag } from 'hooks/useUiFlag';
import useUiConfig from 'hooks/api/getters/useUiConfig/useUiConfig';

interface IDeleteProjectDialogueProps {
project: string;
Expand All @@ -21,6 +24,8 @@ export const DeleteProjectDialogue = ({
const { deleteProject } = useProjectApi();
const { refetch: refetchProjectOverview } = useProjects();
const { setToastData, setToastApiError } = useToast();
const { isEnterprise } = useUiConfig();
const automatedActionsEnabled = useUiFlag('automatedActions');

const onClick = async (e: React.SyntheticEvent) => {
e.preventDefault();
Expand All @@ -45,6 +50,14 @@ export const DeleteProjectDialogue = ({
onClick={onClick}
onClose={onClose}
title='Really delete project'
/>
>
This will irreversibly remove the project, all feature flags
archived in it, all API keys scoped to only this project
<ConditionallyRender
condition={isEnterprise() && automatedActionsEnabled}
show=', and all actions configured for it'
/>
.
</Dialogue>
thomasheartman marked this conversation as resolved.
Show resolved Hide resolved
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -36,41 +36,51 @@ export const DeleteProject = ({
const automatedActionsEnabled = useUiFlag('automatedActions');
const { actions } = useActions(projectId);
const [showDelDialog, setShowDelDialog] = useState(false);
const actionsCount = actions.filter(({ enabled }) => enabled).length;
const navigate = useNavigate();
return (
<StyledContainer>
<p>
Before you can delete a project, you must first archive all the
feature flags associated with it. Keep in mind that deleting a
project will permanently remove all the archived feature flags,
and they cannot be recovered once deleted.
</p>
Tymek marked this conversation as resolved.
Show resolved Hide resolved
<ConditionallyRender
condition={isEnterprise() && automatedActionsEnabled}
condition={featureCount > 0 || actionsCount > 0}
show={
<p>
Additionally, all configured actions for this project
will no longer be executed as they will be permanently
deleted.
</p>
<>
<p>
Before you can delete a project, you must first
archive all the feature flags associated with it.
</p>
<p>
Currently there are{' '}
<strong>{featureCount} feature flags active</strong>
</p>
<ConditionallyRender
condition={
isEnterprise() && automatedActionsEnabled
}
show={
<p>
Currently there are{' '}
<strong>
{actionsCount} enabled actions
</strong>
</p>
}
/>
</>
Copy link
Member

@nunogois nunogois Jul 5, 2024

Choose a reason for hiding this comment

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

I think this can be a bit weird in some edge cases:

  • featureCount === 0
  • actionsCount > 0
  • automatedActionsEnabled === false

We would only show "0 feature flags active", which I'm not sure is the intended behavior. I don't think it matters much, but I would pick one of:

  • Show zeroes: No longer care about the total featureCount and actionsCount totals, show them even if they are 0. This means we simply remove the ConditionallyRender with featureCount > 0 || actionsCount > 0.
  • Don't show zeroes: Add a ConditionallyRender wrapper around each paragraph where we check the individual count for each along with any other conditions:
    • featureCount > 0
    • actionsCount > 0 && isEnterprise() && automatedActionsEnabled

}
/>
<p>
Currently there are{' '}
<strong>{featureCount} feature flags active</strong>
Keep in mind that deleting a project{' '}
<strong>will permanently remove</strong>
<ul>
<li>all archived feature flags in this project</li>
<li>API keys configured to access only this project</li>
Copy link
Contributor

Choose a reason for hiding this comment

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

Depending on how the API key deletion works, we might want to be a little more explicit (here or in the docs), but I don't have a clear suggestion for that yet. Let's see how the rest of this shakes out first.

<ConditionallyRender
condition={isEnterprise() && automatedActionsEnabled}
show={<li>all actions configured for this project</li>}
/>
</ul>
and they <strong>cannot be recovered</strong> once deleted.
</p>
<ConditionallyRender
condition={isEnterprise() && automatedActionsEnabled}
show={
<p>
Currently there are{' '}
<strong>
{actions.filter(({ enabled }) => enabled).length}{' '}
enabled actions
</strong>
</p>
}
/>
<StyledButtonContainer>
<PermissionButton
permission={DELETE_PROJECT}
Expand Down
Loading